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 <noreply@anthropic.com>
This commit is contained in:
@@ -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`
|
||||
|
||||
|
||||
@@ -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<keyof SettingsDefaults>) {
|
||||
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());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -330,4 +330,118 @@ describe('SettingsDefaultsManager', () => {
|
||||
expect(SettingsDefaultsManager.getBool('CLAUDE_MEM_CONTEXT_SHOW_LAST_MESSAGE')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('environment variable overrides', () => {
|
||||
const originalEnv: Record<string, string | undefined> = {};
|
||||
|
||||
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
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user