From 92ae8097dfbb83b643af7c1fa51dfa54d4f9a9c8 Mon Sep 17 00:00:00 2001
From: Jason Woltje <jason.woltje@uscllc.com>
Date: Thu, 5 Feb 2026 13:19:57 -0600
Subject: [PATCH] fix(#101): Remediate code review findings for
 TaskProgressWidget

- Fix CRITICAL: Replace .sort() state mutation with [...tasks].sort()
- Fix CRITICAL: Replace PDA-unfriendly red colors with calm amber tones
- Fix IMPORTANT: Add TaskProgressWidget + ActiveProjectsWidget to WidgetComponentType
- Fix IMPORTANT: Add tests for interval cleanup, HTTP error responses, slice limit
- 3 new tests added (10 total)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 .../components/widgets/TaskProgressWidget.tsx | 14 ++--
 .../__tests__/TaskProgressWidget.test.tsx     | 64 ++++++++++++++++++-
 packages/shared/src/types/widget.types.ts     |  2 +
 3 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/apps/web/src/components/widgets/TaskProgressWidget.tsx b/apps/web/src/components/widgets/TaskProgressWidget.tsx
index 6f54e91..172f8fe 100644
--- a/apps/web/src/components/widgets/TaskProgressWidget.tsx
+++ b/apps/web/src/components/widgets/TaskProgressWidget.tsx
@@ -40,7 +40,7 @@ function getStatusIcon(status: string): React.JSX.Element {
       return <CheckCircle className="w-4 h-4 text-green-500" />;
     case "failed":
     case "killed":
-      return <XCircle className="w-4 h-4 text-red-500" />;
+      return <XCircle className="w-4 h-4 text-amber-500" />;
     default:
       return <Clock className="w-4 h-4 text-gray-400" />;
   }
@@ -73,7 +73,7 @@ function getStatusColor(status: string): string {
       return "bg-green-50 border-green-200 dark:bg-green-950 dark:border-green-800";
     case "failed":
     case "killed":
-      return "bg-red-50 border-red-200 dark:bg-red-950 dark:border-red-800";
+      return "bg-amber-50 border-amber-200 dark:bg-amber-950 dark:border-amber-800";
     default:
       return "bg-gray-50 border-gray-200 dark:bg-gray-900 dark:border-gray-700";
   }
@@ -169,9 +169,9 @@ export function TaskProgressWidget({ id: _id, config: _config }: WidgetProps): R
           </div>
           <div className="text-green-500">Done</div>
         </div>
-        <div className="bg-red-50 dark:bg-red-950 rounded p-2">
-          <div className="text-lg font-bold text-red-600 dark:text-red-400">{stats.failed}</div>
-          <div className="text-red-500">Stopped</div>
+        <div className="bg-amber-50 dark:bg-amber-950 rounded p-2">
+          <div className="text-lg font-bold text-amber-600 dark:text-amber-400">{stats.failed}</div>
+          <div className="text-amber-500">Stopped</div>
         </div>
       </div>
 
@@ -180,7 +180,7 @@ export function TaskProgressWidget({ id: _id, config: _config }: WidgetProps): R
         {tasks.length === 0 ? (
           <div className="text-center text-gray-500 text-sm py-4">No agent tasks in progress</div>
         ) : (
-          tasks
+          [...tasks]
             .sort((a, b) => {
               // Active tasks first, then by spawn time
               const statusOrder: Record<string, number> = {
@@ -217,7 +217,7 @@ export function TaskProgressWidget({ id: _id, config: _config }: WidgetProps): R
                   <span>{getElapsedTime(task.spawnedAt, task.completedAt)}</span>
                 </div>
                 {task.error && (
-                  <div className="text-xs text-red-600 dark:text-red-400 mt-1 truncate">
+                  <div className="text-xs text-amber-600 dark:text-amber-400 mt-1 truncate">
                     {task.error}
                   </div>
                 )}
diff --git a/apps/web/src/components/widgets/__tests__/TaskProgressWidget.test.tsx b/apps/web/src/components/widgets/__tests__/TaskProgressWidget.test.tsx
index f220dc0..47fd72d 100644
--- a/apps/web/src/components/widgets/__tests__/TaskProgressWidget.test.tsx
+++ b/apps/web/src/components/widgets/__tests__/TaskProgressWidget.test.tsx
@@ -2,8 +2,8 @@
  * TaskProgressWidget Component Tests
  */
 
-import { describe, it, expect, vi, beforeEach } from "vitest";
-import { render, screen, waitFor } from "@testing-library/react";
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import { render, screen, waitFor, cleanup } from "@testing-library/react";
 import { TaskProgressWidget } from "../TaskProgressWidget";
 
 const mockFetch = vi.fn();
@@ -14,6 +14,10 @@ describe("TaskProgressWidget", (): void => {
     vi.clearAllMocks();
   });
 
+  afterEach((): void => {
+    cleanup();
+  });
+
   it("should render loading state initially", (): void => {
     mockFetch.mockImplementation(
       () =>
@@ -148,6 +152,62 @@ describe("TaskProgressWidget", (): void => {
     });
   });
 
+  it("should display error for non-ok HTTP responses", async (): Promise<void> => {
+    mockFetch.mockResolvedValueOnce({
+      ok: false,
+      status: 500,
+    } as unknown as Response);
+
+    render(<TaskProgressWidget id="task-progress-1" />);
+
+    await waitFor(() => {
+      expect(screen.getByText(/unable to reach orchestrator/i)).toBeInTheDocument();
+    });
+  });
+
+  it("should clear interval on unmount", async (): Promise<void> => {
+    const clearIntervalSpy = vi.spyOn(global, "clearInterval");
+
+    mockFetch.mockResolvedValue({
+      ok: true,
+      json: () => Promise.resolve([]),
+    } as unknown as Response);
+
+    const { unmount } = render(<TaskProgressWidget id="task-progress-1" />);
+
+    await waitFor(() => {
+      expect(screen.getByText(/no agent tasks in progress/i)).toBeInTheDocument();
+    });
+
+    unmount();
+
+    expect(clearIntervalSpy).toHaveBeenCalled();
+    clearIntervalSpy.mockRestore();
+  });
+
+  it("should limit displayed tasks to 10", async (): Promise<void> => {
+    const mockTasks = Array.from({ length: 15 }, (_, i) => ({
+      agentId: `agent-${String(i)}`,
+      taskId: `SLICE-${String(i).padStart(3, "0")}`,
+      status: "running",
+      agentType: "worker",
+      spawnedAt: new Date().toISOString(),
+    }));
+
+    mockFetch.mockResolvedValueOnce({
+      ok: true,
+      json: () => Promise.resolve(mockTasks),
+    } as unknown as Response);
+
+    render(<TaskProgressWidget id="task-progress-1" />);
+
+    await waitFor(() => {
+      // Only 10 task cards should be rendered despite 15 tasks
+      const workerBadges = screen.getAllByText("Worker");
+      expect(workerBadges).toHaveLength(10);
+    });
+  });
+
   it("should sort active tasks before completed ones", async (): Promise<void> => {
     const mockTasks = [
       {
diff --git a/packages/shared/src/types/widget.types.ts b/packages/shared/src/types/widget.types.ts
index 20e6743..6128e50 100644
--- a/packages/shared/src/types/widget.types.ts
+++ b/packages/shared/src/types/widget.types.ts
@@ -69,6 +69,8 @@ export type WidgetComponentType =
   | "CalendarWidget"
   | "QuickCaptureWidget"
   | "AgentStatusWidget"
+  | "ActiveProjectsWidget"
+  | "TaskProgressWidget"
   | "StatCardWidget"
   | "ChartWidget"
   | "ListWidget"