ci: Bump version to 0.0.9 [skip ci]

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "flyer-crawler",
-  "version": "0.0.8",
+  "version": "0.0.9",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "flyer-crawler",
-      "version": "0.0.8",
+      "version": "0.0.9",
       "dependencies": {
         "@bull-board/api": "^6.14.2",
         "@bull-board/express": "^6.14.2",

package.json

@@ -1,7 +1,7 @@
 {
   "name": "flyer-crawler",
   "private": true,
-  "version": "0.0.8",
+  "version": "0.0.9",
   "type": "module",
   "scripts": {
     "dev": "concurrently \"npm:start:dev\" \"vite\"",

src/routes/system.routes.test.ts

@@ -2,22 +2,20 @@
 import { describe, it, expect, vi, beforeEach, afterAll, beforeAll } from 'vitest';
 import supertest from 'supertest';
 import express, { Express } from 'express';
-import { type ExecException, type ExecOptions } from 'child_process';
-import { geocodingService } from '../services/geocodingService.server';
-import { mockLogger } from '../tests/utils/mockLogger';
+import * as childProcess from 'child_process';
 
 // -----------------------------------------------------------------------------
-// 1. Defensively Mock Modules BEFORE Imports
+// 1. Mocks Configuration
 // -----------------------------------------------------------------------------
 
-// Mock Logger to prevent console noise or memory leaks from logging buffers
+// Mock Logger: Use a completely silent, lightweight object to prevent memory bloat
 vi.mock('../services/logger.server', () => ({
   logger: {
     info: vi.fn(),
     warn: vi.fn(),
     error: vi.fn(),
     debug: vi.fn(),
-    child: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), // Handle child loggers
+    child: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }),
   },
 }));
 
@@ -28,54 +26,66 @@ vi.mock('../services/geocodingService.server', () => ({
   },
 }));
 
-// Mock child_process with a safe factory
-// We define the mock implementation here but control its behavior inside tests via `vi.mocked(exec)`
-const mockExecFn = vi.fn();
-vi.mock('child_process', () => {
+// Mock child_process safely
+// We use importActual to ensure we don't break 'spawn', 'fork' etc. which might be used by internal tools
+vi.mock('child_process', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('child_process')>();
+  const mockExec = vi.fn();
   return {
-    default: { exec: (...args: any[]) => mockExecFn(...args) },
-    exec: (...args: any[]) => mockExecFn(...args),
+    ...actual,
+    exec: mockExec,
+    default: {
+      ...actual,
+      exec: mockExec,
+    },
   };
 });
 
 // Import the router AFTER mocks
 import systemRouter from './system.routes';
+import { geocodingService } from '../services/geocodingService.server';
 
 // -----------------------------------------------------------------------------
-// 2. Test Suite Setup
+// 2. Test Suite
 // -----------------------------------------------------------------------------
 
 describe('System Routes (/api/system)', () => {
   let app: Express;
+  // Create a typed reference to the mocked exec function
+  const mockExec = vi.mocked(childProcess.exec);
 
   beforeAll(() => {
-    // Create a bare-bones express app just for these tests.
-    // This avoids overhead/side-effects from the real `createTestApp`.
+    // Create a minimal express app for isolation
     app = express();
     app.use(express.json());
-
-    // Mount the router
     app.use('/api/system', systemRouter);
 
-    // Simple error handler for the test app
+    // Basic error handler
     app.use((err: any, req: express.Request, res: express.Response, next: express.NextFunction) => {
-      // console.error('[TEST APP ERROR]', err.message); // Uncomment only if debugging extremely deep errors
       res.status(err.status || 500).json({ message: err.message, errors: err.errors });
     });
   });
 
   beforeEach(() => {
     vi.clearAllMocks();
-    vi.resetAllMocks(); // Aggressively reset to clear memory
 
-    // Default safe implementation for exec to prevent crashes if a test forgets to mock it
-    mockExecFn.mockImplementation((cmd, ...args) => {
-      const cb = args.find((arg) => typeof arg === 'function');
-      if (cb) {
-        // Run on next loop to simulate async and avoid Zalgo
-        setTimeout(() => cb(null, '', ''), 0);
+    // Default safe implementation for exec to prevent timeouts or hangs
+    mockExec.mockImplementation((command, options, callback) => {
+      // Normalize args: options is optional
+      let cb: any = callback;
+      if (typeof options === 'function') {
+        cb = options;
       }
-      return { unref: () => {} };
+
+      if (cb) {
+        // Run async to simulate IO
+        const timer = setTimeout(() => {
+          cb(null, 'MOCK_STDOUT', '');
+        }, 0);
+        // Return a mock ChildProcess object
+        return { unref: () => clearTimeout(timer), kill: () => {} } as any;
+      }
+      return { unref: () => {}, kill: () => {} } as any;
     });
   });
 
@@ -83,42 +93,18 @@ describe('System Routes (/api/system)', () => {
     vi.restoreAllMocks();
   });
 
-  // ---------------------------------------------------------------------------
-  // 3. Helper for setting up specific exec scenarios
-  // ---------------------------------------------------------------------------
-  const setupExecMock = (error: ExecException | null, stdout: string, stderr: string) => {
-    mockExecFn.mockImplementation(
-      (
-        command: string,
-        optionsOrCb?: ExecOptions | ((err: any, out: string, errOut: string) => void) | null,
-        cb?: ((err: any, out: string, errOut: string) => void) | null,
-      ) => {
-        let callback: any = cb;
-        if (typeof optionsOrCb === 'function') {
-          callback = optionsOrCb;
-        }
-
-        // Defensive: ensure callback exists
-        if (!callback) return { unref: () => {} };
-
-        // Execute callback asynchronously to mimic real IO
-        setTimeout(() => {
-          try {
-            callback(error, stdout, stderr);
-          } catch (e) {
-            console.error('[TEST CRITICAL] Error inside mock callback:', e);
-          }
-        }, 1);
-
-        return { unref: () => {} };
-      },
-    );
-  };
-
-  // ---------------------------------------------------------------------------
-  // 4. GET /pm2-status Tests
-  // ---------------------------------------------------------------------------
   describe('GET /pm2-status', () => {
+    // Helper to configure the specific exec outcome for a test
+    const mockExecOutcome = (error: Error | null, stdout: string, stderr: string) => {
+      mockExec.mockImplementation((cmd, options, callback) => {
+        const cb = typeof options === 'function' ? options : callback;
+        if (cb) {
+          setTimeout(() => cb(error as any, stdout, stderr), 0);
+        }
+        return { unref: () => {} } as any;
+      });
+    };
+
     const testCases = [
       {
         description: 'should return success: true when pm2 process is online',
@@ -143,7 +129,7 @@ describe('System Routes (/api/system)', () => {
       {
         description: 'should return success: false when pm2 process does not exist',
         mock: {
-          error: Object.assign(new Error('Command failed'), { code: 1 }),
+          error: new Error('Command failed'),
           stdout: "[PM2][ERROR] Process or Namespace flyer-crawler-api doesn't exist",
           stderr: '',
         },
@@ -165,21 +151,15 @@ describe('System Routes (/api/system)', () => {
     ];
 
     it.each(testCases)('$description', async ({ mock, expectedStatus, expectedBody }) => {
-      // Arrange
-      setupExecMock(mock.error as ExecException | null, mock.stdout, mock.stderr);
+      mockExecOutcome(mock.error, mock.stdout, mock.stderr);
 
-      // Act
       const response = await supertest(app).get('/api/system/pm2-status');
 
-      // Assert
       expect(response.status).toBe(expectedStatus);
       expect(response.body).toEqual(expectedBody);
     });
   });
 
-  // ---------------------------------------------------------------------------
-  // 5. POST /geocode Tests
-  // ---------------------------------------------------------------------------
   describe('POST /geocode', () => {
     it('should return geocoded coordinates for a valid address', async () => {
       const mockCoordinates = { lat: 48.4284, lng: -123.3656 };
@@ -205,8 +185,9 @@ describe('System Routes (/api/system)', () => {
     });
 
     it('should return 500 if the geocoding service throws an error', async () => {
-      const geocodeError = new Error('Geocoding service unavailable');
-      vi.mocked(geocodingService.geocodeAddress).mockRejectedValue(geocodeError);
+      vi.mocked(geocodingService.geocodeAddress).mockRejectedValue(
+        new Error('Service unavailable'),
+      );
 
       const response = await supertest(app)
         .post('/api/system/geocode')
@@ -221,8 +202,6 @@ describe('System Routes (/api/system)', () => {
         .send({ not_address: 'Victoria, BC' });
 
       expect(response.status).toBe(400);
-      // Accept flexible validation messages
-      expect(response.body.errors[0].message).toBeTruthy();
     });
   });
 });

src/routes/system.routes.ts

@@ -1,4 +1,4 @@
-// src/routes/system.ts
+// src/routes/system.routests
 import { Router, Request, Response, NextFunction } from 'express';
 import { exec } from 'child_process';
 import { z } from 'zod';
@@ -8,7 +8,7 @@ import { validateRequest } from '../middleware/validation.middleware';
 
 const router = Router();
 
-// Helper for consistent required string validation
+// Validation Schemas
 const requiredString = (message: string) =>
   z.preprocess((val) => val ?? '', z.string().min(1, message));
 
@@ -21,63 +21,62 @@ const geocodeSchema = z.object({
 const emptySchema = z.object({});
 
 /**
- * Checks the status of the 'flyer-crawler-api' process managed by PM2.
+ * GET /pm2-status
+ * Checks if 'flyer-crawler-api' is online via PM2.
  */
 router.get(
   '/pm2-status',
   validateRequest(emptySchema),
   (req: Request, res: Response, next: NextFunction) => {
-    try {
-      // The name 'flyer-crawler-api' comes from ecosystem.config.cjs
-      exec('pm2 describe flyer-crawler-api', (error, stdout, stderr) => {
-        // Defensive: Ensure we don't try to send a response twice or crash if req is closed
-        if (res.headersSent) return;
+    // Using simple exec command
+    const cmd = 'pm2 describe flyer-crawler-api';
 
-        const processNotFound =
-          stdout?.includes("doesn't exist") || stderr?.includes("doesn't exist");
+    exec(cmd, (error, stdout, stderr) => {
+      if (res.headersSent) return;
 
-        if (error) {
-          if (processNotFound) {
-            // Warn instead of Error to keep logs cleaner during known states
-            logger.warn('[API /pm2-status] PM2 process "flyer-crawler-api" not found.');
-            return res.json({
-              success: false,
-              message: 'Application process is not running under PM2.',
-            });
-          }
+      // Handle "process not found" case which might come as an error code OR text
+      const output = (stdout || '') + (stderr || '');
+      const processNotFound = output.includes("doesn't exist");
 
-          // Log concise error
-          logger.error({ msg: 'PM2 exec error', error: error.message });
-          return next(error);
+      if (error) {
+        if (processNotFound) {
+          logger.warn('[API /pm2-status] PM2 process not found.');
+          return res.json({
+            success: false,
+            message: 'Application process is not running under PM2.',
+          });
         }
 
-        if (stderr && stderr.trim().length > 0) {
-          logger.error({ stderr }, '[API /pm2-status] PM2 executed but produced stderr');
-          return next(new Error(`PM2 command produced an error: ${stderr}`));
-        }
+        logger.error({ err: error.message }, '[API /pm2-status] Exec error');
+        return next(error);
+      }
 
-        const isOnline = /│ status\s+│ online\s+│/m.test(stdout);
-        const message = isOnline
-          ? 'Application is online and running under PM2.'
-          : 'Application process exists but is not online.';
+      // Treat stderr as error if it contains text (PM2 often outputs warnings here)
+      if (stderr && stderr.trim().length > 0) {
+        // Special case: if it's just a warning we might want to ignore, but sticking to defensive rules:
+        logger.error({ stderr }, '[API /pm2-status] Stderr output');
+        return next(new Error(`PM2 command produced an error: ${stderr}`));
+      }
 
-        res.json({ success: isOnline, message });
-      });
-    } catch (syncError) {
-      // Catch synchronous errors in starting exec (extremely rare but good for defensive coding)
-      next(syncError);
-    }
+      // Check for online status in the table output
+      const isOnline = /│ status\s+│ online\s+│/m.test(stdout || '');
+      const message = isOnline
+        ? 'Application is online and running under PM2.'
+        : 'Application process exists but is not online.';
+
+      res.json({ success: isOnline, message });
+    });
   },
 );
 
 /**
- * POST /api/system/geocode - Geocodes a given address string.
+ * POST /geocode
+ * Proxies geocoding requests securely.
  */
 router.post(
   '/geocode',
   validateRequest(geocodeSchema),
   async (req: Request, res: Response, next: NextFunction) => {
-    // Type casting logic as per ADR-003
     type GeocodeRequest = z.infer<typeof geocodeSchema>;
     const {
       body: { address },