test: backfill mockable coverage for live-only behavior + guard against it

scripts/checks/no-unit-blocks-in-live-e2e.ts

@@ -0,0 +1,124 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+//
+// Guard: pure unit blocks must not hide inside test/e2e/live/** files.
+//
+// vitest.config.ts only collects test/e2e/live/**/*.test.ts when live E2E is
+// enabled (NEMOCLAW_RUN_LIVE_E2E=1). On PR CI that flag is false, so the entire
+// file is uncollected — including any `describe(...)` unit block embedded in it.
+// Such blocks are dead weight on PR CI: they read like coverage but never run
+// where they could. This is exactly how two mockable regressions stayed
+// unguarded (the skill-agent classifiers and the openclaw TUI-correlation
+// logic, the latter saved only by a lucky root-level duplicate).
+//
+// Convention this guard enforces: inside test/e2e/live/**, the vitest unit
+// primitive `it(` is banned. Live cases are declared with `test` — directly, or
+// (more often) through a gate wrapper assigned from `shouldRunLiveE2E() ? test
+// : test.skip` / `test.skipIf(!shouldRunLiveE2E())`, sometimes grouped under
+// `describe.sequential(...)`. A live case never needs `it(`; when `it(` appears
+// in a live file it is invariably a pure-unit block someone parked there (as
+// happened with the skill-agent and messaging classifier blocks). Such a block
+// is dead on PR CI and belongs in an importable module + a PR-collected test
+// (root test/**, a co-located src/**/*.test.ts, or test/e2e/support/**).
+//
+// We deliberately do NOT try to flag bare `test(` unit cases: a live test that
+// uses module-level helpers legitimately reads as `test("...", async () => …)`
+// with no fixture, and is syntactically indistinguishable from a unit case. The
+// `it(` ban is the reliable, zero-false-positive line.
+
+import { existsSync, readdirSync, readFileSync, statSync } from "node:fs";
+import path from "node:path";
+import { fileURLToPath } from "node:url";
+
+const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../..");
+const LIVE_DIR = path.join(REPO_ROOT, "test", "e2e", "live");
+const TEST_FILE_PATTERN = /\.(?:test|spec)\.(?:[cm]?[jt]s)$/;
+// Match the vitest unit primitive `it(` — including `it.each(`, `it.only(`,
+// `it.skip(`, etc. — as a call at a statement boundary. The leading boundary
+// (line start or whitespace) prevents matching inside a custom identifier, and
+// requiring a call paren after the optional member keeps non-call references
+// from matching.
+const IT_PRIMITIVE_PATTERN =
+  /(?:^|[\s;{(])it(?:\.(?:each|only|skip|todo|fails|concurrent|sequential))?\s*\(/;
+
+export type LiveUnitBlockViolation = {
+  readonly file: string;
+  readonly line: number;
+  readonly text: string;
+};
+
+function toRepoPath(absPath: string): string {
+  return path.relative(REPO_ROOT, absPath).split(path.sep).join("/");
+}
+
+function* walkFiles(dir: string): Generator<string> {
+  if (!existsSync(dir)) return;
+  for (const entry of readdirSync(dir)) {
+    const absPath = path.join(dir, entry);
+    const stats = statSync(absPath);
+    if (stats.isDirectory()) {
+      yield* walkFiles(absPath);
+    } else if (stats.isFile() && TEST_FILE_PATTERN.test(entry)) {
+      yield absPath;
+    }
+  }
+}
+
+export function findLiveUnitBlocks(source: string, file: string): LiveUnitBlockViolation[] {
+  const violations: LiveUnitBlockViolation[] = [];
+  const lines = source.split(/\r\n|\r|\n/);
+  for (let i = 0; i < lines.length; i += 1) {
+    const text = lines[i] ?? "";
+    const trimmed = text.trimStart();
+    // Skip import lines (`import { it, test } from "vitest"`) and comments.
+    if (trimmed.startsWith("import ") || trimmed.startsWith("//") || trimmed.startsWith("*")) {
+      continue;
+    }
+    if (IT_PRIMITIVE_PATTERN.test(text)) {
+      violations.push({ file, line: i + 1, text: trimmed });
+    }
+  }
+  return violations;
+}
+
+export function collectLiveUnitBlocks(dir = LIVE_DIR): LiveUnitBlockViolation[] {
+  return [...walkFiles(dir)]
+    .flatMap((absPath) => findLiveUnitBlocks(readFileSync(absPath, "utf-8"), toRepoPath(absPath)))
+    .sort((a, b) => a.file.localeCompare(b.file) || a.line - b.line);
+}
+
+export function formatViolations(violations: readonly LiveUnitBlockViolation[]): string {
+  const out = [
+    "Live E2E unit-block guard failed.",
+    "",
+    "These test/e2e/live/** files use the vitest unit primitive it(...). That glob",
+    "is only collected when NEMOCLAW_RUN_LIVE_E2E=1, so an it(...) block never runs",
+    "on PR CI — it looks like coverage but guards nothing. Live cases use test(...)",
+    "(directly or via a gate wrapper); it(...) in a live file is always a pure-unit",
+    "block parked in the wrong place.",
+    "",
+    "Fix: extract the helper under test into an importable module (src/** or",
+    "test/e2e/support/**) and move the it(...) block to a PR-collected project",
+    "(root test/**/*.test.ts, a co-located src/**/*.test.ts, or test/e2e/support/**).",
+    "Keep the live test importing the shared helper.",
+    "",
+  ];
+  for (const v of violations) {
+    out.push(`- ${v.file}:${v.line}  ${v.text}`);
+  }
+  return out.join("\n");
+}
+
+function main(): void {
+  const violations = collectLiveUnitBlocks();
+  if (violations.length > 0) {
+    console.error(formatViolations(violations));
+    process.exitCode = 1;
+    return;
+  }
+  console.log("Live E2E unit-block guard passed: no it(...) blocks in test/e2e/live/**.");
+}
+
+if (fileURLToPath(import.meta.url) === path.resolve(process.argv[1] ?? "")) {
+  main();
+}

scripts/checks/run.ts

@@ -61,6 +61,11 @@ const CHECKS: readonly CheckCommand[] = [
     command: TSX,
     args: ["scripts/checks/test-title-style.ts"],
   },
+  {
+    name: "no-unit-blocks-in-live-e2e",
+    command: TSX,
+    args: ["scripts/checks/no-unit-blocks-in-live-e2e.ts"],
+  },
 ];
 
 function main(): void {

scripts/nemoclaw-start.sh

@@ -1907,6 +1907,9 @@ node_options_has_require() {
   local token
   local tokens=()
   IFS=$' \t\n' read -r -a tokens <<<"${NODE_OPTIONS:-}"
+  # Iterating "${tokens[@]}" on an empty array trips `set -u` on bash 3.2
+  # (macOS default); guard so the local unit harnesses run there too.
+  [ "${#tokens[@]}" -gt 0 ] || return 1
   for token in "${tokens[@]}"; do
     if [ "$previous" = "--require" ] && [ "$token" = "$wanted" ]; then
       return 0
@@ -2014,7 +2017,7 @@ validate_nemoclaw_tmp_permissions() {
     [ -n "$_target" ] && _dynamic_targets+=("$_target")
   done < <(messaging_runtime_preload_targets)
 
-  validate_tmp_permissions "$_SANDBOX_SAFETY_NET" "$_PROXY_FIX_SCRIPT" "$_NEMOTRON_FIX_SCRIPT" "$_WS_FIX_SCRIPT" "$_SECCOMP_GUARD_SCRIPT" "$_CIAO_GUARD_SCRIPT" "${_dynamic_targets[@]}"
+  validate_tmp_permissions "$_SANDBOX_SAFETY_NET" "$_PROXY_FIX_SCRIPT" "$_NEMOTRON_FIX_SCRIPT" "$_WS_FIX_SCRIPT" "$_SECCOMP_GUARD_SCRIPT" "$_CIAO_GUARD_SCRIPT" "${_dynamic_targets[@]+"${_dynamic_targets[@]}"}"
 }
 
 verify_messaging_runtime_secret_scans() {
@@ -2374,7 +2377,7 @@ start_auto_pair() {
   if [ "$(id -u)" -eq 0 ]; then
     run_prefix=("${STEP_DOWN_PREFIX_SANDBOX[@]}")
   fi
-  OPENCLAW_BIN="$OPENCLAW" nohup "${run_prefix[@]}" python3 -u - <<'PYAUTOPAIR' >>/tmp/auto-pair.log 2>&1 &
+  OPENCLAW_BIN="$OPENCLAW" nohup "${run_prefix[@]+"${run_prefix[@]}"}" python3 -u - <<'PYAUTOPAIR' >>/tmp/auto-pair.log 2>&1 &
 import json
 import importlib.util
 import os

src/lib/actions/sandbox/snapshot.test.ts

@@ -750,6 +750,19 @@ describe("runSandboxSnapshot", () => {
     expect(output).toContain("2 snapshot(s). Restore with:");
   });
 
+  it("prints create, list, and restore usage for the bare help branch", async () => {
+    const consoleLog = vi.spyOn(console, "log").mockImplementation(() => {});
+    const { runSandboxSnapshot } = await import("./snapshot");
+
+    await runSandboxSnapshot("alpha", { kind: "help" });
+
+    const output = consoleLog.mock.calls.flat().join("\n");
+    expect(output).toContain("Usage:");
+    expect(output).toContain("alpha snapshot create");
+    expect(output).toContain("alpha snapshot list");
+    expect(output).toContain("alpha snapshot restore");
+  });
+
   it("restores the latest snapshot into the source sandbox", async () => {
     const consoleLog = vi.spyOn(console, "log").mockImplementation(() => {});
     getLatestBackupMock.mockReturnValue({

src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact-test-helpers.ts

@@ -0,0 +1,25 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+// Test harness helpers for whatsapp-qr-compact.test.ts. The Module._load hook
+// keeps the test body linear; the routing decision itself reuses the runtime's
+// exported resolvePatchedModule so the test exercises real production logic
+// rather than a re-implemented copy.
+
+import { resolvePatchedModule } from "./whatsapp-qr-compact";
+
+/**
+ * Build a Module._load wrapper identical to the runtime's: for the given
+ * absolute path it returns `patchedModule`, otherwise a bare object, then
+ * delegates to the runtime's resolvePatchedModule so patching happens only for
+ * qrcode-shaped requests and never leaks onto passthrough modules.
+ */
+export function makeQrcodeLoadHook(
+  absolutePath: string,
+  patchedModule: unknown,
+): (request: unknown, ...rest: unknown[]) => unknown {
+  return function (request: unknown, ..._rest: unknown[]) {
+    const loaded = request === absolutePath ? patchedModule : {};
+    return resolvePatchedModule(request, loaded);
+  };
+}

src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.test.ts

@@ -0,0 +1,163 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+// Unit coverage for the WhatsApp compact-QR preload's pure shape-detect and
+// patch helpers (NemoClaw#4522 wrong-package-patch regression class). The live
+// whatsapp-qr-compact E2E only asserts terminal row counts against the real
+// upstream renderer; these tests pin the load-hook contract hermetically with
+// fake module objects so no real qrcode / qrcode-terminal dependency is needed.
+
+import { describe, expect, it, vi } from "vitest";
+
+import {
+  isQrcodePackage,
+  isQrcodeTerminalPackage,
+  patchQrcode,
+  patchQrcodeTerminal,
+} from "./whatsapp-qr-compact";
+import { makeQrcodeLoadHook } from "./whatsapp-qr-compact-test-helpers";
+
+// A fake of the `qrcode` package main: has its OWN toString + create().
+function makeQrcodeFake() {
+  const calls: Array<{ text: unknown; opts: unknown; cb: unknown }> = [];
+  const mod = {
+    calls,
+    create() {
+      return {};
+    },
+    toString(text: unknown, opts?: unknown, cb?: unknown) {
+      calls.push({ text, opts, cb });
+      return "QR";
+    },
+  };
+  return mod;
+}
+
+// A fake of the `qrcode-terminal` package: has generate(), no create().
+function makeQrcodeTerminalFake() {
+  const calls: Array<{ text: unknown; opts: unknown; cb: unknown }> = [];
+  const mod = {
+    calls,
+    generate(text: unknown, opts?: unknown, cb?: unknown) {
+      calls.push({ text, opts, cb });
+    },
+  };
+  return mod;
+}
+
+describe("isQrcodePackage (#4522)", () => {
+  it("detects the qrcode package main by own toString + create", () => {
+    expect(isQrcodePackage(makeQrcodeFake())).toBe(true);
+  });
+
+  it("does not match a lookalike submodule that only has create()", () => {
+    // qrcode's internal lib/core/qrcode.js exposes create() but only the
+    // inherited Object.prototype.toString — it must NOT be patched.
+    const submodule = {
+      create() {
+        return {};
+      },
+    };
+    expect(isQrcodePackage(submodule)).toBe(false);
+  });
+
+  it("does not match qrcode-terminal (has generate, no create)", () => {
+    expect(isQrcodePackage(makeQrcodeTerminalFake())).toBe(false);
+  });
+});
+
+describe("isQrcodeTerminalPackage (#4522)", () => {
+  it("detects qrcode-terminal by own generate and absent create", () => {
+    expect(isQrcodeTerminalPackage(makeQrcodeTerminalFake())).toBe(true);
+  });
+
+  it("does not match the qrcode package (has create)", () => {
+    expect(isQrcodeTerminalPackage(makeQrcodeFake())).toBe(false);
+  });
+});
+
+describe("patchQrcode (#4522)", () => {
+  it("forces small:true only for terminal renders", () => {
+    const mod = makeQrcodeFake();
+    patchQrcode(mod);
+    mod.toString("payload", { type: "terminal" });
+    expect(mod.calls[0].opts).toEqual({ type: "terminal", small: true });
+  });
+
+  it.each(["svg", "png", "utf8"])("leaves type=%s options untouched", (type) => {
+    const mod = makeQrcodeFake();
+    patchQrcode(mod);
+    mod.toString("payload", { type });
+    expect(mod.calls[0].opts).toEqual({ type });
+    expect((mod.calls[0].opts as Record<string, unknown>).small).toBeUndefined();
+  });
+
+  it("does not mutate the caller-supplied options object", () => {
+    const mod = makeQrcodeFake();
+    patchQrcode(mod);
+    const opts = { type: "terminal" };
+    mod.toString("payload", opts);
+    expect(opts).toEqual({ type: "terminal" });
+  });
+
+  it("preserves the toString(text, cb) signature", () => {
+    const mod = makeQrcodeFake();
+    patchQrcode(mod);
+    const cb = vi.fn();
+    mod.toString("payload", cb);
+    expect(mod.calls[0].cb).toBe(cb);
+    // No opts object was supplied, so nothing is forced.
+    expect(mod.calls[0].opts).toEqual({});
+  });
+
+  it("is idempotent: double-patch does not re-wrap", () => {
+    const mod = makeQrcodeFake();
+    patchQrcode(mod);
+    const wrappedOnce = mod.toString;
+    patchQrcode(mod);
+    expect(mod.toString).toBe(wrappedOnce);
+    // And forcing still works exactly once.
+    mod.toString("payload", { type: "terminal" });
+    expect(mod.calls[0].opts).toEqual({ type: "terminal", small: true });
+  });
+});
+
+describe("patchQrcodeTerminal (#4522)", () => {
+  it("forces small:true on generate", () => {
+    const mod = makeQrcodeTerminalFake();
+    patchQrcodeTerminal(mod);
+    mod.generate("payload", {});
+    expect(mod.calls[0].opts).toEqual({ small: true });
+  });
+
+  it("is idempotent: double-patch does not re-wrap", () => {
+    const mod = makeQrcodeTerminalFake();
+    patchQrcodeTerminal(mod);
+    const wrappedOnce = mod.generate;
+    patchQrcodeTerminal(mod);
+    expect(mod.generate).toBe(wrappedOnce);
+  });
+});
+
+describe("Module._load hook path-segment matching (#4522)", () => {
+  it('patches import("qrcode")\'s resolved absolute path', async () => {
+    // Simulate the real load hook: install a Module._load wrapper identical to
+    // the runtime's, then require by an ABSOLUTE resolved path (as import()
+    // bottoms out at) and confirm the returned module got the compact patch.
+    const Module = (await import("node:module")).default as unknown as {
+      _load: (...args: unknown[]) => unknown;
+    };
+    const qrcodeFake = makeQrcodeFake();
+    const absolutePath = "/tmp/app/node_modules/qrcode/lib/index.js";
+    const origLoad = Module._load;
+    Module._load = makeQrcodeLoadHook(absolutePath, qrcodeFake);
+    try {
+      const loaded = Module._load(absolutePath) as ReturnType<typeof makeQrcodeFake>;
+      expect(loaded).toBe(qrcodeFake);
+      loaded.toString("payload", { type: "terminal" });
+      expect(loaded.calls[0].opts).toEqual({ type: "terminal", small: true });
+    } finally {
+      Module._load = origLoad;
+    }
+  });
+});