CORE-510: Orphaned app dev worker processes

.github/workflows/build.yaml

@@ -49,6 +49,46 @@ jobs:
       - name: Lint
         run: pnpm lint
 
+  test:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Setup Node.js environment
+        uses: actions/setup-node@v4
+        with:
+          node-version: 20
+
+      - uses: pnpm/action-setup@v2
+        name: Install pnpm
+        with:
+          version: 9
+          run_install: false
+
+      - name: Get pnpm store directory
+        shell: bash
+        run: |
+          echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV
+
+      - uses: actions/cache@v4
+        name: Setup pnpm cache
+        with:
+          path: ${{ env.STORE_PATH }}
+          key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}
+          restore-keys: |
+            ${{ runner.os }}-pnpm-store-
+
+      - name: Install dependencies
+        run: pnpm i --frozen-lockfile
+
+      - name: Build project
+        run: pnpm build
+
+      - name: Run tests
+        run: pnpm test
+
   build:
     runs-on: ubuntu-latest
 

package.json

@@ -8,6 +8,8 @@
   "scripts": {
     "build": "pnpm -r build",
     "lint": "eslint --fix packages/**/*.{ts,js,json}",
+    "test": "pnpm -r test",
+    "test:watch": "pnpm -r test:watch",
     "release": "pnpm build && bumpp packages/*/package.json --commit \"release: v\" --push --tag && pnpm -r release"
   },
   "workspaces": {
@@ -17,6 +19,7 @@
   },
   "devDependencies": {
     "@types/node": "^18.18.0",
+    "@vitest/ui": "^2.1.8",
     "@youcan/lint": "^3.0.6",
     "bumpp": "^9.2.0",
     "dotenv": "^16.3.1",
@@ -27,7 +30,8 @@
     "rollup-plugin-node-externals": "^6.1.1",
     "rollup-plugin-typescript2": "^0.36.0",
     "tslib": "^2.8.1",
-    "typescript": "5.1.6"
+    "typescript": "5.1.6",
+    "vitest": "^2.1.8"
   },
   "pnpm": {
     "overrides": {

packages/app/lib/cli/commands/app/dev.test.ts

@@ -0,0 +1,196 @@
+import type { Worker } from '@youcan/cli-kit';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import Dev from './dev';
+
+vi.mock('@/util/app-loader', () => ({
+  load: vi.fn().mockResolvedValue({
+    root: '/test/app',
+    config: { id: 'test-app' },
+    webs: [],
+    extensions: [],
+  }),
+}));
+
+vi.mock('@youcan/cli-kit', () => ({
+  Session: {
+    authenticate: vi.fn().mockResolvedValue({}),
+  },
+  Tasks: {
+    run: vi.fn().mockResolvedValue({ workers: [] }),
+  },
+  UI: {
+    renderDevOutput: vi.fn(),
+  },
+  System: {
+    getPortOrNextOrRandom: vi.fn().mockResolvedValue(3000),
+  },
+  Services: {
+    Cloudflared: vi.fn(),
+  },
+  Filesystem: {
+    writeJsonFile: vi.fn(),
+  },
+  Path: {
+    join: vi.fn(),
+  },
+  Cli: {
+    Command: class MockCommand {
+      controller = { abort: vi.fn(), signal: new AbortController().signal };
+      output = { wait: vi.fn() };
+    },
+  },
+  Env: {},
+  Http: {},
+  Worker: {
+    Interface: class MockInterface {
+      async run() {}
+      async boot() {}
+      async cleanup() {}
+    },
+    Abstract: class MockAbstract {
+      async run() {}
+      async boot() {}
+      async cleanup() {}
+    },
+  },
+}));
+
+vi.spyOn(process, 'exit').mockImplementation(() => undefined as never);
+const mockOnce = vi.spyOn(process, 'once').mockImplementation(() => process);
+
+describe('dev Command', () => {
+  let devCommand: Dev;
+  let mockWorkers: Worker.Interface[];
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+
+    mockWorkers = [
+      {
+        run: vi.fn(),
+        boot: vi.fn(),
+        cleanup: vi.fn().mockResolvedValue(undefined),
+      },
+      {
+        run: vi.fn(),
+        boot: vi.fn(),
+        cleanup: vi.fn().mockResolvedValue(undefined),
+      },
+    ];
+
+    devCommand = new Dev([], {});
+    (devCommand as any).workers = mockWorkers;
+  });
+
+  describe('setupExitHandlers', () => {
+    it('should register signal handlers for graceful shutdown', () => {
+      (devCommand as any).setupExitHandlers();
+
+      expect(mockOnce).toHaveBeenCalledWith('SIGINT', expect.any(Function));
+      expect(mockOnce).toHaveBeenCalledWith('SIGTERM', expect.any(Function));
+      expect(mockOnce).toHaveBeenCalledWith('SIGQUIT', expect.any(Function));
+      expect(mockOnce).toHaveBeenCalledWith('exit', expect.any(Function));
+    });
+  });
+
+  describe('cleanup functionality', () => {
+    it('should cleanup all workers when cleanupAndExit is called', async () => {
+      const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+      (devCommand as any).setupExitHandlers();
+
+      const sigintCall = mockOnce.mock.calls.find(call => call[0] === 'SIGINT');
+      const cleanupAndExit = sigintCall?.[1];
+
+      if (cleanupAndExit) {
+        await cleanupAndExit();
+      }
+
+      expect(consoleSpy).toHaveBeenCalledWith('Shutting down...');
+      expect(mockWorkers[0].cleanup).toHaveBeenCalled();
+      expect(mockWorkers[1].cleanup).toHaveBeenCalled();
+
+      consoleSpy.mockRestore();
+    });
+
+    it('should handle worker cleanup errors gracefully', async () => {
+      const failingWorker = {
+        run: vi.fn(),
+        boot: vi.fn(),
+        cleanup: vi.fn().mockRejectedValue(new Error('Cleanup failed')),
+      };
+      (devCommand as any).workers = [failingWorker];
+
+      const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+      (devCommand as any).setupExitHandlers();
+      const sigintCall = mockOnce.mock.calls.find(call => call[0] === 'SIGINT');
+      const cleanupAndExit = sigintCall?.[1];
+
+      if (cleanupAndExit) {
+        await expect(cleanupAndExit()).resolves.toBeUndefined();
+      }
+
+      expect(failingWorker.cleanup).toHaveBeenCalled();
+
+      consoleSpy.mockRestore();
+    });
+  });
+
+  describe('reloadWorkers', () => {
+    it('should cleanup existing workers before reload', async () => {
+      const mockController = {
+        abort: vi.fn(),
+        signal: new AbortController().signal,
+      };
+      (devCommand as any).controller = mockController;
+      (devCommand as any).app = {
+        network_config: { app_port: 3000 },
+      };
+
+      (devCommand as any).syncAppConfig = vi.fn().mockResolvedValue(undefined);
+      (devCommand as any).prepareDevProcesses = vi.fn().mockResolvedValue([]);
+      (devCommand as any).runWorkers = vi.fn().mockResolvedValue(undefined);
+
+      await (devCommand as any).reloadWorkers();
+
+      expect(mockWorkers[0].cleanup).toHaveBeenCalled();
+      expect(mockWorkers[1].cleanup).toHaveBeenCalled();
+    });
+  });
+
+  describe('runWorkers', () => {
+    it('should store workers in instance variable', async () => {
+      const newWorkers = [
+        {
+          run: vi.fn().mockResolvedValue(undefined),
+          boot: vi.fn(),
+          cleanup: vi.fn(),
+        },
+      ];
+
+      await (devCommand as any).runWorkers(newWorkers);
+
+      expect((devCommand as any).workers).toBe(newWorkers);
+      expect(newWorkers[0].run).toHaveBeenCalled();
+    });
+  });
+
+  describe('hotkey handlers', () => {
+    it('should cleanup workers when q key is pressed', async () => {
+      const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+      const hotKeys = (devCommand as any).hotKeys;
+      const quitHandler = hotKeys.find((key: any) => key.keyboardKey === 'q')?.handler;
+
+      if (quitHandler) {
+        await quitHandler();
+      }
+
+      expect(consoleSpy).toHaveBeenCalledWith('Shutting down...');
+      expect(mockWorkers[0].cleanup).toHaveBeenCalled();
+      expect(mockWorkers[1].cleanup).toHaveBeenCalled();
+
+      consoleSpy.mockRestore();
+    });
+  });
+});

packages/app/lib/cli/commands/app/dev.ts

@@ -22,14 +22,19 @@ class Dev extends AppCommand {
   }
 
   private setupExitHandlers() {
-    const cleanupAndExit = () => {
+    const cleanupAndExit = async () => {
       try {
+        console.log('Shutting down...');
+
+        if (this.workers.length > 0) {
+          await Promise.allSettled(this.workers.map(worker => worker.cleanup()));
+        }
+
         if (this.controller) {
           this.controller.abort();
         }
         this.workers = [];
 
-        console.log('Shutting down...');
         setTimeout(() => {
           process.exit(0);
         }, 100);
@@ -43,7 +48,10 @@ class Dev extends AppCommand {
     process.once('SIGTERM', cleanupAndExit);
     process.once('SIGQUIT', cleanupAndExit);
 
-    process.once('exit', () => {
+    process.once('exit', async () => {
+      if (this.workers.length > 0) {
+        await Promise.allSettled(this.workers.map(worker => worker.cleanup()));
+      }
       if (this.controller) {
         this.controller.abort();
       }
@@ -59,13 +67,19 @@ class Dev extends AppCommand {
     {
       keyboardKey: 'q',
       description: 'quit',
-      handler: () => {
+      handler: async () => {
         try {
+          console.log('Shutting down...');
+
+          if (this.workers.length > 0) {
+            await Promise.allSettled(this.workers.map(worker => worker.cleanup()));
+          }
+
           if (this.controller) {
             this.controller.abort();
           }
           this.workers = [];
-          console.log('Shutting down...');
+
           setTimeout(() => {
             process.exit(0);
           }, 100);
@@ -134,6 +148,11 @@ class Dev extends AppCommand {
   async reloadWorkers() {
     this.controller = new AbortController();
 
+    if (this.workers.length > 0) {
+      await Promise.allSettled(this.workers.map(worker => worker.cleanup()));
+      this.workers = [];
+    }
+
     const networkConfig = this.app.network_config;
     this.app = await load();
     this.app.network_config = networkConfig;
@@ -144,6 +163,8 @@ class Dev extends AppCommand {
   }
 
   private async runWorkers(workers: Worker.Interface[]): Promise<void> {
+    this.workers = workers;
+
     await Promise.all(workers.map(worker => worker.run())).catch((_) => { });
   }