coderabbit comments

Author: dblythy

package.json

@@ -61,7 +61,7 @@
     "winston": "3.19.0",
     "winston-daily-rotate-file": "5.0.0",
     "ws": "8.18.2",
-    "zod": "^4.3.6"
+    "zod": "4.3.6"
   },
   "devDependencies": {
     "@actions/core": "3.0.0",

spec/Options/loaders/envLoader.spec.js

@@ -106,7 +106,7 @@ describe('envLoader', () => {
 
   it('coerces object env vars from JSON', () => {
     const schema = z.object({
-      push: option(z.record(z.unknown()).optional(), {
+      push: option(z.record(z.string(), z.unknown()).optional(), {
         env: 'PARSE_SERVER_PUSH',
         help: 'Push config',
       }),

spec/Options/loaders/fileLoader.spec.js

@@ -29,6 +29,32 @@ describe('fileLoader', () => {
     }
   });
 
+  it('throws for empty apps array', () => {
+    const fs = require('fs');
+    const emptyAppsPath = path.join(configDir, 'CLIConfigEmptyApps.json');
+    fs.writeFileSync(emptyAppsPath, JSON.stringify({ apps: [] }));
+    try {
+      expect(() => loadFromFile(emptyAppsPath)).toThrow(
+        'The "apps" array must contain at least one configuration'
+      );
+    } finally {
+      fs.unlinkSync(emptyAppsPath);
+    }
+  });
+
+  it('throws for apps that is not an array', () => {
+    const fs = require('fs');
+    const nonArrayAppsPath = path.join(configDir, 'CLIConfigNonArrayApps.json');
+    fs.writeFileSync(nonArrayAppsPath, JSON.stringify({ apps: {} }));
+    try {
+      expect(() => loadFromFile(nonArrayAppsPath)).toThrow(
+        'The "apps" property must be an array'
+      );
+    } finally {
+      fs.unlinkSync(nonArrayAppsPath);
+    }
+  });
+
   it('resolves relative paths', () => {
     const result = loadFromFile('./spec/configs/CLIConfig.json');
     expect(result.arg1).toBe('my_app');

src/Config.js

@@ -63,13 +63,13 @@ export class Config {
       })
     );
 
-    const cachedConfig = AppCache.get(this.appId);
+    const cachedConfig = AppCache.get(this.applicationId);
     if (cachedConfig) {
       const updatedConfig = { ...cachedConfig };
       asyncKeys.forEach(key => {
         updatedConfig[key] = this[key];
       });
-      AppCache.put(this.appId, updatedConfig);
+      AppCache.put(this.applicationId, updatedConfig);
     }
   }
 

src/Options/loaders/cliLoader.ts

@@ -53,7 +53,7 @@ export function extractCliOptions(
  * Checks if a Zod schema field is optional or has a default value.
  */
 function isOptionalOrDefaulted(schema: z.ZodTypeAny): boolean {
-  if (schema instanceof z.ZodOptional || schema instanceof z.ZodNullable) {
+  if (schema instanceof z.ZodOptional) {
     return true;
   }
   if (schema instanceof z.ZodDefault) {

src/Options/loaders/envLoader.ts

@@ -42,8 +42,12 @@ function setNestedValue(obj: Record<string, unknown>, path: string[], value: unk
   let current: Record<string, unknown> = obj;
   for (let i = 0; i < path.length - 1; i++) {
     const key = path[i];
-    if (current[key] === undefined || typeof current[key] !== 'object') {
+    if (current[key] === undefined) {
       current[key] = {};
+    } else if (current[key] === null || typeof current[key] !== 'object' || Array.isArray(current[key])) {
+      throw new Error(
+        `Environment variable path collision at "${path.slice(0, i + 1).join('.')}": expected an object but found ${Array.isArray(current[key]) ? 'an array' : typeof current[key]}`
+      );
     }
     current = current[key] as Record<string, unknown>;
   }

src/Options/loaders/fileLoader.ts

@@ -18,6 +18,12 @@ export function loadFromFile(filePath: string): Record<string, unknown> {
   let options: Record<string, unknown>;
 
   if (jsonConfig.apps) {
+    if (!Array.isArray(jsonConfig.apps)) {
+      throw new Error('The "apps" property must be an array');
+    }
+    if (jsonConfig.apps.length === 0) {
+      throw new Error('The "apps" array must contain at least one configuration');
+    }
     if (jsonConfig.apps.length > 1) {
       throw new Error('Multiple apps are not supported');
     }

src/Options/loaders/mergeConfig.ts

@@ -32,6 +32,13 @@ function deepMerge(
   source: Record<string, unknown>
 ): void {
   for (const key of Object.keys(source)) {
+    if (key === '__proto__' || key === 'constructor' || key === 'prototype') {
+      continue;
+    }
+    if (!Object.prototype.hasOwnProperty.call(source, key)) {
+      continue;
+    }
+
     const sourceVal = source[key];
     const targetVal = target[key];
 
@@ -44,6 +51,8 @@ function deepMerge(
         targetVal as Record<string, unknown>,
         sourceVal as Record<string, unknown>
       );
+    } else if (isPlainObject(sourceVal)) {
+      target[key] = structuredClone(sourceVal);
     } else {
       target[key] = sourceVal;
     }

src/Options/schemaUtils.ts

@@ -146,14 +146,14 @@ export function coerceValue(value: string, fieldSchema: z.ZodTypeAny): unknown {
 
   if (innerType instanceof z.ZodArray) {
     if (typeof value === 'string') {
-      // Try JSON array first, fall back to CSV
       try {
         const parsed = JSON.parse(value);
         if (Array.isArray(parsed)) return parsed;
       } catch {
-        // Not JSON, treat as CSV
+        // Not valid JSON
       }
-      return value.split(',');
+      // Return original value so Zod validation fails upstream
+      return value;
     }
     return value;
   }
@@ -171,7 +171,8 @@ export function coerceValue(value: string, fieldSchema: z.ZodTypeAny): unknown {
 
   if (innerType instanceof z.ZodUnion) {
     // For union types, try each branch
-    const options = (innerType as z.ZodUnion<[z.ZodTypeAny, ...z.ZodTypeAny[]]>)._def.options;
+    const unionDef = innerType as z.ZodUnion<[z.ZodTypeAny, ...z.ZodTypeAny[]]>;
+    const options = (unionDef as any)._zod?.def?.options ?? (unionDef as any)._def.options;
     for (const opt of options) {
       const inner = unwrapType(opt);
       // Skip function types for string coercion

src/Options/schemas/AccountLockoutOptions.ts

@@ -5,7 +5,7 @@ export const AccountLockoutOptionsSchema = z
   .object({
     duration: option(
       z.number()
-        .min(1, 'Account lockout duration should be greater than 0')
+        .gt(0, 'Account lockout duration should be greater than 0')
         .max(99999, 'Account lockout duration should be less than 100000')
         .optional(),
       {