From 109fb1b506d6495798549075778ab0e4b857d70f Mon Sep 17 00:00:00 2001 From: Alex Newman Date: Fri, 6 Feb 2026 03:03:28 -0500 Subject: [PATCH] MAESTRO: Cherry-pick PR #712 - respect environment variables with correct priority Adds applyEnvOverrides() method to SettingsDefaultsManager ensuring env vars take highest priority over file and default settings. Configuration priority is now: env > file > defaults. Co-Authored-By: Claude Opus 4.6 --- Auto Run Docs/PR-Triage/PR-Triage-11.md | 3 +- src/shared/SettingsDefaultsManager.ts | 32 ++++- .../shared/settings-defaults-manager.test.ts | 114 ++++++++++++++++++ 3 files changed, 143 insertions(+), 6 deletions(-) diff --git a/Auto Run Docs/PR-Triage/PR-Triage-11.md b/Auto Run Docs/PR-Triage/PR-Triage-11.md index 9c7b66f8..5f9bb3e3 100644 --- a/Auto Run Docs/PR-Triage/PR-Triage-11.md +++ b/Auto Run Docs/PR-Triage/PR-Triage-11.md @@ -20,7 +20,8 @@ Standalone bug fixes that don't group neatly into other phases. - [x] Review PR #634 (`fix: respect CLAUDE_CONFIG_DIR for plugin paths (#626)` by @Kuroakira). Files: 14 files across paths, hooks, and services. Steps: (1) `gh pr checkout 634` (2) Review — should use `CLAUDE_CONFIG_DIR` env var instead of hardcoded `~/.claude/` path (3) Large changeset — verify it doesn't break default behavior when env var is not set (4) Run `npm run build` (5) If clean: `gh pr merge 634 --rebase --delete-branch` - **Cherry-picked onto main.** PR had build artifact merge conflicts (plugin/scripts/* — minified bundles), but all 6 source changes applied cleanly. Adds `MARKETPLACE_ROOT` constant to `paths.ts`, updates `HealthMonitor.ts`, `worker-utils.ts`, `BranchManager.ts`, `CursorHooksInstaller.ts`, and `ObservationCompiler.ts` to use centralized constants instead of hardcoded `homedir() + '.claude'` paths. Backwards compatible — defaults to `~/.claude` when `CLAUDE_CONFIG_DIR` is not set. TypeScript compilation passes for all modified files. Build failure is pre-existing (dompurify dep). Fixes #626. -- [ ] Review PR #712 (`Fix environment variables` by @cjpeterein). Files: SettingsDefaultsManager.ts + build artifacts + tests. Steps: (1) `gh pr checkout 712` (2) Review — what env var fix? Check the diff for specifics (3) Run `npm run build` (4) If clean and focused: `gh pr merge 712 --rebase --delete-branch` +- [x] Review PR #712 (`Fix environment variables` by @cjpeterein). Files: SettingsDefaultsManager.ts + build artifacts + tests. Steps: (1) `gh pr checkout 712` (2) Review — what env var fix? Check the diff for specifics (3) Run `npm run build` (4) If clean and focused: `gh pr merge 712 --rebase --delete-branch` + - **Cherry-picked onto main.** Adds `applyEnvOverrides()` method ensuring env vars take highest priority: `env > file > defaults`. Applied to all 3 return paths in `loadFromFile()` (missing file, normal load, error fallback). 6 test cases added and passing. Enables runtime configuration overrides for containerized deployments and CI/CD without modifying settings files. Build artifacts had merge conflicts (minified bundles) — source changes applied cleanly. - [ ] Review PR #524 (`fix: add minimum bun version check to smart-install.js` by @quicktime). File: `plugin/scripts/smart-install.js`. Steps: (1) `gh pr checkout 524` (2) Review version check logic — what minimum version? Is it still relevant? (3) If clean: `gh pr merge 524 --rebase --delete-branch` diff --git a/src/shared/SettingsDefaultsManager.ts b/src/shared/SettingsDefaultsManager.ts index d3b34ac6..0f386cb2 100644 --- a/src/shared/SettingsDefaultsManager.ts +++ b/src/shared/SettingsDefaultsManager.ts @@ -131,10 +131,29 @@ export class SettingsDefaultsManager { return value === 'true' || value === true; } + /** + * Apply environment variable overrides to settings + * Environment variables take highest priority over file and defaults + */ + private static applyEnvOverrides(settings: SettingsDefaults): SettingsDefaults { + const result = { ...settings }; + for (const key of Object.keys(this.DEFAULTS) as Array) { + if (process.env[key] !== undefined) { + result[key] = process.env[key]!; + } + } + return result; + } + /** * Load settings from file with fallback to defaults - * Returns merged settings with defaults as fallback - * Handles all errors (missing file, corrupted JSON, permissions) by returning defaults + * Returns merged settings with proper priority: process.env > settings file > defaults + * Handles all errors (missing file, corrupted JSON, permissions) gracefully + * + * Configuration Priority: + * 1. Environment variables (highest priority) + * 2. Settings file (~/.claude-mem/settings.json) + * 3. Default values (lowest priority) */ static loadFromFile(settingsPath: string): SettingsDefaults { try { @@ -151,7 +170,8 @@ export class SettingsDefaultsManager { } catch (error) { console.warn('[SETTINGS] Failed to create settings file, using in-memory defaults:', settingsPath, error); } - return defaults; + // Still apply env var overrides even when file doesn't exist + return this.applyEnvOverrides(defaults); } const settingsData = readFileSync(settingsPath, 'utf-8'); @@ -181,10 +201,12 @@ export class SettingsDefaultsManager { } } - return result; + // Apply environment variable overrides (highest priority) + return this.applyEnvOverrides(result); } catch (error) { console.warn('[SETTINGS] Failed to load settings, using defaults:', settingsPath, error); - return this.getAllDefaults(); + // Still apply env var overrides even on error + return this.applyEnvOverrides(this.getAllDefaults()); } } } diff --git a/tests/shared/settings-defaults-manager.test.ts b/tests/shared/settings-defaults-manager.test.ts index e82301b0..240202fc 100644 --- a/tests/shared/settings-defaults-manager.test.ts +++ b/tests/shared/settings-defaults-manager.test.ts @@ -330,4 +330,118 @@ describe('SettingsDefaultsManager', () => { expect(SettingsDefaultsManager.getBool('CLAUDE_MEM_CONTEXT_SHOW_LAST_MESSAGE')).toBe(false); }); }); + + describe('environment variable overrides', () => { + const originalEnv: Record = {}; + + beforeEach(() => { + // Save original env values + originalEnv.CLAUDE_MEM_WORKER_PORT = process.env.CLAUDE_MEM_WORKER_PORT; + originalEnv.CLAUDE_MEM_MODEL = process.env.CLAUDE_MEM_MODEL; + originalEnv.CLAUDE_MEM_LOG_LEVEL = process.env.CLAUDE_MEM_LOG_LEVEL; + }); + + afterEach(() => { + // Restore original env values + if (originalEnv.CLAUDE_MEM_WORKER_PORT === undefined) { + delete process.env.CLAUDE_MEM_WORKER_PORT; + } else { + process.env.CLAUDE_MEM_WORKER_PORT = originalEnv.CLAUDE_MEM_WORKER_PORT; + } + if (originalEnv.CLAUDE_MEM_MODEL === undefined) { + delete process.env.CLAUDE_MEM_MODEL; + } else { + process.env.CLAUDE_MEM_MODEL = originalEnv.CLAUDE_MEM_MODEL; + } + if (originalEnv.CLAUDE_MEM_LOG_LEVEL === undefined) { + delete process.env.CLAUDE_MEM_LOG_LEVEL; + } else { + process.env.CLAUDE_MEM_LOG_LEVEL = originalEnv.CLAUDE_MEM_LOG_LEVEL; + } + }); + + it('should prioritize env var over file setting', () => { + // File has port 12345, env var has 54321 + const fileSettings = { + CLAUDE_MEM_WORKER_PORT: '12345', + }; + writeFileSync(settingsPath, JSON.stringify(fileSettings)); + process.env.CLAUDE_MEM_WORKER_PORT = '54321'; + + const result = SettingsDefaultsManager.loadFromFile(settingsPath); + + expect(result.CLAUDE_MEM_WORKER_PORT).toBe('54321'); + }); + + it('should prioritize env var over default', () => { + // No file, env var set + process.env.CLAUDE_MEM_WORKER_PORT = '99999'; + + const result = SettingsDefaultsManager.loadFromFile(settingsPath); + + expect(result.CLAUDE_MEM_WORKER_PORT).toBe('99999'); + }); + + it('should use file setting when env var is not set', () => { + const fileSettings = { + CLAUDE_MEM_WORKER_PORT: '11111', + }; + writeFileSync(settingsPath, JSON.stringify(fileSettings)); + delete process.env.CLAUDE_MEM_WORKER_PORT; + + const result = SettingsDefaultsManager.loadFromFile(settingsPath); + + expect(result.CLAUDE_MEM_WORKER_PORT).toBe('11111'); + }); + + it('should apply env var override even on file parse error', () => { + writeFileSync(settingsPath, 'invalid json {{{'); + process.env.CLAUDE_MEM_WORKER_PORT = '88888'; + + const result = SettingsDefaultsManager.loadFromFile(settingsPath); + + expect(result.CLAUDE_MEM_WORKER_PORT).toBe('88888'); + }); + + it('should apply multiple env var overrides', () => { + const fileSettings = { + CLAUDE_MEM_WORKER_PORT: '12345', + CLAUDE_MEM_MODEL: 'file-model', + CLAUDE_MEM_LOG_LEVEL: 'DEBUG', + }; + writeFileSync(settingsPath, JSON.stringify(fileSettings)); + + process.env.CLAUDE_MEM_WORKER_PORT = '54321'; + process.env.CLAUDE_MEM_MODEL = 'env-model'; + // LOG_LEVEL not set in env, should use file value + + const result = SettingsDefaultsManager.loadFromFile(settingsPath); + + expect(result.CLAUDE_MEM_WORKER_PORT).toBe('54321'); + expect(result.CLAUDE_MEM_MODEL).toBe('env-model'); + expect(result.CLAUDE_MEM_LOG_LEVEL).toBe('DEBUG'); // From file + }); + + it('should document priority: env > file > defaults', () => { + // This test documents the expected priority order + const defaults = SettingsDefaultsManager.getAllDefaults(); + + // Set file to something different from default + const fileSettings = { + CLAUDE_MEM_WORKER_PORT: '22222', // Different from default 37777 + }; + writeFileSync(settingsPath, JSON.stringify(fileSettings)); + + // Set env to something different from both + process.env.CLAUDE_MEM_WORKER_PORT = '33333'; + + const result = SettingsDefaultsManager.loadFromFile(settingsPath); + + // Priority check: + // Default is 37777, file is 22222, env is 33333 + // Result should be env (33333) because env > file > default + expect(defaults.CLAUDE_MEM_WORKER_PORT).toBe('37777'); // Confirm default + expect(result.CLAUDE_MEM_WORKER_PORT).toBe('33333'); // Env wins + }); + }); });