From cf7f316e71ced4d9bfde1759a5cd08f54a4488c2 Mon Sep 17 00:00:00 2001
From: Stefanos Mousafeiris <msfstef@gmail.com>
Date: Thu, 27 Jun 2024 13:02:29 +0300
Subject: [PATCH] fix(cli): Fix boolean flag defaults not being respected
 (#1405)

Fixes https://github.com/electric-sql/electric/issues/1404

Was overriding default with environment variable value, but not checking
if environment variable was actually set
---
 .changeset/blue-dots-rush.md       |  5 +++++
 components/cli/src/config.ts       |  2 +-
 components/cli/test/config.test.ts | 33 ++++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/blue-dots-rush.md

diff --git a/.changeset/blue-dots-rush.md b/.changeset/blue-dots-rush.md
new file mode 100644
index 00000000..fba758cb
--- /dev/null
+++ b/.changeset/blue-dots-rush.md
@@ -0,0 +1,5 @@
+---
+"@electric-sql/cli": patch
+---
+
+Fix boolean flag defaults not being respected.
diff --git a/components/cli/src/config.ts b/components/cli/src/config.ts
index 82a83911..fdbc0de3 100644
--- a/components/cli/src/config.ts
+++ b/components/cli/src/config.ts
@@ -110,7 +110,7 @@ export function getConfigValue<K extends ConfigOptionName>(
   // Then check if the option was passed as an environment variable
   const envName = name.startsWith('ELECTRIC_') ? name : `ELECTRIC_${name}`
   const envVal = process.env[envName]
-  if (configOptions[name].valueType === Boolean) {
+  if (envName in process.env && configOptions[name].valueType === Boolean) {
     return (!!envVal &&
       !['f', 'false', '0', '', 'no'].includes(
         envVal?.toLocaleLowerCase()
diff --git a/components/cli/test/config.test.ts b/components/cli/test/config.test.ts
index 539bcd18..6f1579db 100644
--- a/components/cli/test/config.test.ts
+++ b/components/cli/test/config.test.ts
@@ -1,5 +1,38 @@
 import test from 'ava'
 import { getConfigValue, redactConfigSecrets } from '../src/config'
+import { configOptions } from '../src/config-options'
+
+const origEnv = { ...process.env }
+const origConfigOptions = { ...configOptions }
+test.beforeEach(() => {
+  // restore environment and config options
+  process.env = origEnv
+  Object.assign(configOptions, origConfigOptions)
+})
+
+test('getConfigValue respects boolean flag defaults', async (t) => {
+  const flagWithTrueDefault = '_MOCK_TRUE_DEFAULT'
+  const flagWithFalseDefault = '_MOCK_FALSE_DEFAULT'
+
+  configOptions[flagWithTrueDefault] = {
+    valueType: Boolean,
+    defaultVal: true,
+  }
+
+  configOptions[flagWithFalseDefault] = {
+    valueType: Boolean,
+    defaultVal: false,
+  }
+
+  t.is(getConfigValue(flagWithTrueDefault), true)
+  t.is(getConfigValue(flagWithFalseDefault), false)
+
+  // ensure environment overrides default
+  process.env[`ELECTRIC_${flagWithTrueDefault}`] = 'false'
+  process.env[`ELECTRIC_${flagWithFalseDefault}`] = 'true'
+  t.is(getConfigValue(flagWithTrueDefault), false)
+  t.is(getConfigValue(flagWithFalseDefault), true)
+})
 
 test('getConfigValue can capture `ELECTRIC_` prefixed CLI opitons', async (t) => {
   const image = getConfigValue('ELECTRIC_IMAGE', { image: 'electric:test' })