From 880919c77eb7a7659681734a50dac339cfaf695b Mon Sep 17 00:00:00 2001
From: Jason Woltje <jason.woltje@uscllc.com>
Date: Thu, 5 Feb 2026 18:54:52 -0600
Subject: [PATCH] fix(#338): Add tests to verify runner jobs interval cleanup

- Add test verifying clearInterval is called in finally block
- Add test verifying interval is cleared even when stream throws error
- Prevents memory leaks from leaked intervals

The clearInterval was already present in the codebase at line 409 of
runner-jobs.service.ts. These tests provide explicit verification
of the cleanup behavior.

Refs #338

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 .../runner-jobs/runner-jobs.service.spec.ts   | 86 ++++++++++++++++++-
 1 file changed, 83 insertions(+), 3 deletions(-)

diff --git a/apps/api/src/runner-jobs/runner-jobs.service.spec.ts b/apps/api/src/runner-jobs/runner-jobs.service.spec.ts
index 39b12bf..c53ace7 100644
--- a/apps/api/src/runner-jobs/runner-jobs.service.spec.ts
+++ b/apps/api/src/runner-jobs/runner-jobs.service.spec.ts
@@ -608,14 +608,11 @@ describe("RunnerJobsService", () => {
       const jobId = "job-123";
       const workspaceId = "workspace-123";
 
-      let closeHandler: (() => void) | null = null;
-
       const mockRes = {
         write: vi.fn(),
         end: vi.fn(),
         on: vi.fn((event: string, handler: () => void) => {
           if (event === "close") {
-            closeHandler = handler;
             // Immediately trigger close to break the loop
             setTimeout(() => handler(), 10);
           }
@@ -638,6 +635,89 @@ describe("RunnerJobsService", () => {
       expect(mockRes.end).toHaveBeenCalled();
     });
 
+    it("should call clearInterval in finally block to prevent memory leaks", async () => {
+      const jobId = "job-123";
+      const workspaceId = "workspace-123";
+
+      // Spy on global setInterval and clearInterval
+      const mockIntervalId = 12345;
+      const setIntervalSpy = vi
+        .spyOn(global, "setInterval")
+        .mockReturnValue(mockIntervalId as never);
+      const clearIntervalSpy = vi.spyOn(global, "clearInterval").mockImplementation(() => {});
+
+      const mockRes = {
+        write: vi.fn(),
+        end: vi.fn(),
+        on: vi.fn(),
+        writableEnded: false,
+      };
+
+      // Mock job to complete immediately
+      mockPrismaService.runnerJob.findUnique
+        .mockResolvedValueOnce({
+          id: jobId,
+          status: RunnerJobStatus.RUNNING,
+        })
+        .mockResolvedValueOnce({
+          id: jobId,
+          status: RunnerJobStatus.COMPLETED,
+        });
+
+      mockPrismaService.jobEvent.findMany.mockResolvedValue([]);
+
+      await service.streamEvents(jobId, workspaceId, mockRes as never);
+
+      // Verify setInterval was called for keep-alive ping
+      expect(setIntervalSpy).toHaveBeenCalled();
+
+      // Verify clearInterval was called with the interval ID to prevent memory leak
+      expect(clearIntervalSpy).toHaveBeenCalledWith(mockIntervalId);
+
+      // Cleanup spies
+      setIntervalSpy.mockRestore();
+      clearIntervalSpy.mockRestore();
+    });
+
+    it("should clear interval even when stream throws an error", async () => {
+      const jobId = "job-123";
+      const workspaceId = "workspace-123";
+
+      // Spy on global setInterval and clearInterval
+      const mockIntervalId = 54321;
+      const setIntervalSpy = vi
+        .spyOn(global, "setInterval")
+        .mockReturnValue(mockIntervalId as never);
+      const clearIntervalSpy = vi.spyOn(global, "clearInterval").mockImplementation(() => {});
+
+      const mockRes = {
+        write: vi.fn(),
+        end: vi.fn(),
+        on: vi.fn(),
+        writableEnded: false,
+      };
+
+      mockPrismaService.runnerJob.findUnique.mockResolvedValueOnce({
+        id: jobId,
+        status: RunnerJobStatus.RUNNING,
+      });
+
+      // Simulate a fatal error during event polling
+      mockPrismaService.jobEvent.findMany.mockRejectedValue(new Error("Fatal database failure"));
+
+      // The method should throw but still clean up
+      await expect(service.streamEvents(jobId, workspaceId, mockRes as never)).rejects.toThrow(
+        "Fatal database failure"
+      );
+
+      // Verify clearInterval was called even on error (via finally block)
+      expect(clearIntervalSpy).toHaveBeenCalledWith(mockIntervalId);
+
+      // Cleanup spies
+      setIntervalSpy.mockRestore();
+      clearIntervalSpy.mockRestore();
+    });
+
     // ERROR RECOVERY TESTS - Issue #187
 
     it("should support resuming stream from lastEventId", async () => {