fix: remove merge-level type guard, rely on fb()/fbArray() for fallback

The type guard blocked valid user overrides when the system config had
a malformed type (e.g., system 'true' string vs user true boolean).
Since fb()/fbArray() already handle normalization fallback, the guard
was redundant for normalized fields.

For raw pass-through fields (featureFlagOverrides, viewedSplashScreens,
serverSshHost), add targeted inline type validation with system fallback
instead of a blanket merge-level guard.

Adds test: valid user overrides win even when system has wrong type.

Author: ibetitsmike

src/node/config.test.ts

@@ -612,7 +612,7 @@ describe("Config", () => {
       // User's intentional [] should win over system defaults
       expect(loaded.hiddenModels).toEqual([]);
     });
-    it("type-mismatched user values do not displace system defaults", () => {
+    it("type-mismatched user values fall back to system via normalization", () => {
       writeSystemConfig({
         mdnsAdvertisementEnabled: true,
         updateChannel: "stable",
@@ -629,12 +629,47 @@ describe("Config", () => {
 
       const loaded = config.loadConfigOrDefault();
 
-      // System defaults survive because type-mismatched user values are rejected
+      // System defaults survive: fb() falls back for booleans/strings,
+      // inline type check falls back for raw objects (featureFlagOverrides)
       expect(loaded.mdnsAdvertisementEnabled).toBe(true);
       expect(loaded.updateChannel).toBe("stable");
       expect(loaded.featureFlagOverrides).toEqual({ flagA: "on" });
     });
 
+    it("valid user overrides win even when system has wrong type", () => {
+      writeSystemConfig({
+        // Admin typo: "true" (string) instead of true (boolean)
+        mdnsAdvertisementEnabled: "true" as unknown as boolean,
+        projects: [],
+      });
+      writeUserConfig({
+        // User has the correct type — should not be blocked
+        mdnsAdvertisementEnabled: false,
+        projects: [],
+      });
+
+      const loaded = config.loadConfigOrDefault();
+
+      // User's valid boolean wins over system's malformed string
+      expect(loaded.mdnsAdvertisementEnabled).toBe(false);
+    });
+    it("non-object user value does not displace system object (runtimeEnablement)", () => {
+      writeSystemConfig({
+        runtimeEnablement: { docker: false },
+        projects: [],
+      });
+      writeUserConfig({
+        // Malformed: string instead of object
+        runtimeEnablement: "oops" as unknown as Record<string, unknown>,
+        projects: [],
+      });
+
+      const loaded = config.loadConfigOrDefault();
+
+      // System object survives — merge rejects non-object user value for object fields
+      expect(loaded.runtimeEnablement).toEqual({ docker: false });
+    });
+
     it("same-type but invalid user values fall back to system via normalization", () => {
       writeSystemConfig({
         updateChannel: "stable",

src/node/config.ts

@@ -336,33 +336,26 @@ export class Config {
       if (userVal === undefined) continue;
 
       const systemVal = systemRecord[key];
-
-      // Type guard: reject user values whose JSON type fundamentally differs
-      // from the system default. Without this, a user value like "oops" for a
-      // boolean field would overwrite the valid system boolean, only to be
-      // stripped to undefined by normalization — silently losing the baseline.
-      if (systemVal !== undefined) {
-        const sysIsArr = Array.isArray(systemVal);
-        const usrIsArr = Array.isArray(userVal);
-        if (sysIsArr !== usrIsArr || (!sysIsArr && typeof systemVal !== typeof userVal)) {
-          continue;
+      // For scalar/primitive fields, no type guard here — fb()/fbArray() in
+      // the normalization section reject invalid merged values and fall back
+      // to the raw system value. A blanket type guard would block valid user
+      // overrides when the system config itself has a typo (e.g., system
+      // "true" string vs user true boolean).
+      //
+      // For object fields, we protect the system object from being replaced
+      // by a non-object user value (e.g., runtimeEnablement: "oops"). This
+      // is safe because a non-object can never be a valid override for an
+      // object-valued setting.
+      if (systemVal != null && typeof systemVal === "object" && !Array.isArray(systemVal)) {
+        if (userVal != null && typeof userVal === "object" && !Array.isArray(userVal)) {
+          // LIMITATION: This is a shallow key-level merge — user sub-values replace
+          // system sub-values without per-value validation. E.g., a user setting
+          // featureFlagOverrides.flagA = "bogus" overwrites the admin's "on". This
+          // is accepted because validation lives in consumers (getFeatureFlagOverride),
+          // not the config merge layer, and the set of valid values varies per key.
+          mergedRecord[key] = { ...systemVal, ...userVal };
         }
-      }
-
-      if (
-        systemVal != null &&
-        typeof systemVal === "object" &&
-        !Array.isArray(systemVal) &&
-        userVal != null &&
-        typeof userVal === "object" &&
-        !Array.isArray(userVal)
-      ) {
-        // LIMITATION: This is a shallow key-level merge — user sub-values replace
-        // system sub-values without per-value validation. E.g., a user setting
-        // featureFlagOverrides.flagA = "bogus" overwrites the admin's "on". This
-        // is accepted because validation lives in consumers (getFeatureFlagOverride),
-        // not the config merge layer, and the set of valid values varies per key.
-        mergedRecord[key] = { ...systemVal, ...userVal };
+        // else: user value is non-object — skip it, system object survives.
       } else {
         mergedRecord[key] = userVal;
       }
@@ -618,7 +611,11 @@ export class Config {
             parsed.mdnsServiceName,
             systemParsed?.mdnsServiceName
           ),
-          serverSshHost: parsed.serverSshHost,
+          serverSshHost: fb(
+            parseOptionalNonEmptyString,
+            parsed.serverSshHost,
+            systemParsed?.serverSshHost
+          ),
           serverAuthGithubOwner: fb(
             parseOptionalNonEmptyString,
             parsed.serverAuthGithubOwner,
@@ -629,7 +626,11 @@ export class Config {
             parsed.defaultProjectDir,
             systemParsed?.defaultProjectDir
           ),
-          viewedSplashScreens: parsed.viewedSplashScreens,
+          viewedSplashScreens: fbArray(
+            parseOptionalStringArray,
+            parsed.viewedSplashScreens,
+            systemParsed?.viewedSplashScreens
+          ),
           layoutPresets,
           taskSettings,
           muxGatewayEnabled,
@@ -639,7 +640,14 @@ export class Config {
           agentAiDefaults,
           // Legacy fields are still parsed and returned for downgrade compatibility.
           subagentAiDefaults: legacySubagentAiDefaults,
-          featureFlagOverrides: parsed.featureFlagOverrides,
+          // featureFlagOverrides is a raw pass-through (no normalizer) — validate
+          // that the merged value is actually a plain object before accepting it.
+          featureFlagOverrides:
+            parsed.featureFlagOverrides != null &&
+            typeof parsed.featureFlagOverrides === "object" &&
+            !Array.isArray(parsed.featureFlagOverrides)
+              ? parsed.featureFlagOverrides
+              : systemParsed?.featureFlagOverrides,
           useSSH2Transport: fb(
             parseOptionalBoolean,
             parsed.useSSH2Transport,