From c68b541b6f236e147aa7809a2067582957b89d43 Mon Sep 17 00:00:00 2001
From: Jason Woltje <jason.woltje@uscllc.com>
Date: Thu, 5 Feb 2026 13:26:21 -0600
Subject: [PATCH] fix(#226): Remediate code review findings for E2E tests

- Fix CRITICAL: Remove unused imports (Test, TestingModule, CleanupService)
- Fix CRITICAL: Remove unused mockValkeyService declaration
- Fix IMPORTANT: Rename misleading test describe/names to match actual behavior
- Fix IMPORTANT: Verify spawned agents exist before kill-all assertion

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 .../integration/agent-lifecycle.e2e-spec.ts      | 16 ++--------------
 .../tests/integration/killswitch.e2e-spec.ts     | 12 ++++++++----
 2 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/apps/orchestrator/tests/integration/agent-lifecycle.e2e-spec.ts b/apps/orchestrator/tests/integration/agent-lifecycle.e2e-spec.ts
index 6aa4494..ebe70b5 100644
--- a/apps/orchestrator/tests/integration/agent-lifecycle.e2e-spec.ts
+++ b/apps/orchestrator/tests/integration/agent-lifecycle.e2e-spec.ts
@@ -9,7 +9,6 @@
  * Covers issue #226 (ORCH-125)
  */
 import { describe, it, expect, beforeEach, vi } from "vitest";
-import { Test, TestingModule } from "@nestjs/testing";
 import { ConfigService } from "@nestjs/config";
 import { AgentSpawnerService } from "../../src/spawner/agent-spawner.service";
 import { AgentLifecycleService } from "../../src/spawner/agent-lifecycle.service";
@@ -24,17 +23,6 @@ describe("E2E: Full Agent Lifecycle", () => {
   let lifecycleService: AgentLifecycleService;
   let queueService: QueueService;
 
-  const mockValkeyService = {
-    getAgentState: vi.fn(),
-    setAgentState: vi.fn(),
-    updateAgentStatus: vi.fn(),
-    publishEvent: vi.fn(),
-    getConnection: vi.fn().mockReturnValue({
-      host: "localhost",
-      port: 6379,
-    }),
-  };
-
   const mockConfigService = {
     get: vi.fn((key: string, defaultValue?: unknown) => {
       const config: Record<string, unknown> = {
@@ -83,8 +71,8 @@ describe("E2E: Full Agent Lifecycle", () => {
     );
   });
 
-  describe("Happy path: spawn → running → completed", () => {
-    it("should complete a full agent lifecycle from spawn to completion", async () => {
+  describe("Happy path: spawn → queue → track", () => {
+    it("should spawn an agent, register it, and queue the task", async () => {
       // Step 1: Spawn agent
       const spawnResult = await controller.spawn({
         taskId: "e2e-task-001",
diff --git a/apps/orchestrator/tests/integration/killswitch.e2e-spec.ts b/apps/orchestrator/tests/integration/killswitch.e2e-spec.ts
index 7904cb0..b8f19b2 100644
--- a/apps/orchestrator/tests/integration/killswitch.e2e-spec.ts
+++ b/apps/orchestrator/tests/integration/killswitch.e2e-spec.ts
@@ -8,7 +8,6 @@
  */
 import { describe, it, expect, beforeEach, vi } from "vitest";
 import { KillswitchService } from "../../src/killswitch/killswitch.service";
-import { CleanupService } from "../../src/killswitch/cleanup.service";
 import { AgentSpawnerService } from "../../src/spawner/agent-spawner.service";
 import { AgentsController } from "../../src/api/agents/agents.controller";
 import { QueueService } from "../../src/queue/queue.service";
@@ -90,9 +89,10 @@ describe("E2E: Killswitch", () => {
 
   describe("Kill all agents", () => {
     it("should kill all active agents", async () => {
-      // Spawn multiple agents
+      // Spawn multiple agents to verify they exist before kill-all
+      const spawned = [];
       for (let i = 0; i < 3; i++) {
-        await controller.spawn({
+        const result = await controller.spawn({
           taskId: `kill-all-test-${String(i)}`,
           agentType: "worker",
           context: {
@@ -101,9 +101,13 @@ describe("E2E: Killswitch", () => {
             workItems: [`US-${String(i)}`],
           },
         });
+        spawned.push(result);
       }
 
-      // Kill all
+      // Verify agents were spawned
+      expect(spawnerService.listAgentSessions()).toHaveLength(3);
+
+      // Kill all (mock returns hardcoded result matching spawn count)
       const result = await controller.killAllAgents();
 
       expect(result.total).toBe(3);