diff --git a/main.js b/main.js index 41ee0493..402e04ed 100644 --- a/main.js +++ b/main.js @@ -38,6 +38,7 @@ const cleanPtyEnv = Object.fromEntries( const { discoverShellProfiles, getShellProfiles, resolveShell, isWindows, isWslShell, windowsToWslPath, shellArgs, quoteArgvForShell } = require('./shell-profiles'); const { startScheduler } = require('./schedule-runner'); const { encodeProjectPath } = require('./encode-project-path'); +const { resolveEffectiveSettings } = require('./resolve-effective-settings'); // --- Auto-updater (only in packaged builds) --- @@ -854,16 +855,7 @@ ipcMain.handle('get-shell-profiles', () => { ipcMain.handle('get-effective-settings', (_event, projectPath) => { const global = getSetting('global') || {}; const project = projectPath ? (getSetting('project:' + projectPath) || {}) : {}; - const effective = { ...SETTING_DEFAULTS }; - for (const key of Object.keys(SETTING_DEFAULTS)) { - if (global[key] !== undefined && global[key] !== null) { - effective[key] = global[key]; - } - if (project[key] !== undefined && project[key] !== null) { - effective[key] = project[key]; - } - } - return effective; + return resolveEffectiveSettings(SETTING_DEFAULTS, global, project); }); // --- IPC: get-active-sessions --- diff --git a/resolve-effective-settings.js b/resolve-effective-settings.js new file mode 100644 index 00000000..8e3c556d --- /dev/null +++ b/resolve-effective-settings.js @@ -0,0 +1,28 @@ +/** + * Merge saved settings over defaults for a project. + * + * Scopes apply narrowest-last: defaults, then global, then project. + * + * Only `undefined` — the key was never saved at that scope — falls through to + * the next-broader value. An explicit `null` is a real, deliberate choice and + * wins like any other value. The settings panel relies on this: it persists + * permissionMode's "Default (none)" option as `value || null`, so `null` there + * means "pass no --permission-mode flag", not "unset". + * + * Extracted from main.js's get-effective-settings handler so this rule can be + * unit-tested without booting Electron. + */ +function resolveEffectiveSettings(defaults, global = {}, project = {}) { + const effective = { ...defaults }; + for (const key of Object.keys(defaults)) { + if (global[key] !== undefined) { + effective[key] = global[key]; + } + if (project[key] !== undefined) { + effective[key] = project[key]; + } + } + return effective; +} + +module.exports = { resolveEffectiveSettings }; diff --git a/test/resolve-effective-settings.test.js b/test/resolve-effective-settings.test.js new file mode 100644 index 00000000..ed24d7ed --- /dev/null +++ b/test/resolve-effective-settings.test.js @@ -0,0 +1,102 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { resolveEffectiveSettings } = require('../resolve-effective-settings'); + +// Mirrors the shape of main.js's SETTING_DEFAULTS for the keys that matter here. +const DEFAULTS = { + permissionMode: null, + dangerouslySkipPermissions: false, + worktree: false, + visibleSessionCount: 5, + shellProfile: 'auto', +}; + +test('returns the defaults when nothing has been saved', () => { + assert.deepEqual(resolveEffectiveSettings(DEFAULTS, {}, {}), DEFAULTS); +}); + +test('project scope overrides global, which overrides defaults', () => { + const effective = resolveEffectiveSettings( + DEFAULTS, + { permissionMode: 'plan', visibleSessionCount: 10 }, + { permissionMode: 'acceptEdits' }, + ); + assert.equal(effective.permissionMode, 'acceptEdits', 'project wins over global'); + assert.equal(effective.visibleSessionCount, 10, 'global still applies where project is silent'); + assert.equal(effective.shellProfile, 'auto', 'untouched keys keep their default'); +}); + +test('a project can narrow permissionMode back to Default over a global mode', () => { + // The settings panel saves the "Default (none)" option as `value || null`, so + // an explicit null is how a user says "prompt for every action, pass no + // --permission-mode flag". It must beat a broader-scope mode. + const effective = resolveEffectiveSettings( + DEFAULTS, + { permissionMode: 'bypassPermissions' }, + { permissionMode: null }, + ); + assert.equal(effective.permissionMode, null, + 'an explicitly saved null must not fall back to the global mode'); +}); + +test('an explicit global null beats a non-null default', () => { + // With a null default this is invisible, so pin it against a default that is + // not null — otherwise any future non-null SETTING_DEFAULTS value silently + // becomes unreachable. + const effective = resolveEffectiveSettings( + { permissionMode: 'acceptEdits' }, + { permissionMode: null }, + {}, + ); + assert.equal(effective.permissionMode, null, + 'an explicitly saved null must override a non-null default'); +}); + +test('undefined means "never saved" and falls through', () => { + const effective = resolveEffectiveSettings( + DEFAULTS, + { permissionMode: 'plan' }, + { permissionMode: undefined }, + ); + assert.equal(effective.permissionMode, 'plan', + 'an absent project key must not shadow the global value'); +}); + +test('null and undefined are not conflated', () => { + const withNull = resolveEffectiveSettings(DEFAULTS, { permissionMode: 'plan' }, { permissionMode: null }); + const withUndefined = resolveEffectiveSettings(DEFAULTS, { permissionMode: 'plan' }, {}); + assert.notEqual(withNull.permissionMode, withUndefined.permissionMode, + 'an explicit null and an absent key must resolve differently'); +}); + +test('other falsy values are preserved', () => { + const effective = resolveEffectiveSettings( + { worktree: true, visibleSessionCount: 5, shellProfile: 'auto' }, + {}, + { worktree: false, visibleSessionCount: 0, shellProfile: '' }, + ); + assert.equal(effective.worktree, false, 'false must override a true default'); + assert.equal(effective.visibleSessionCount, 0, '0 must override a non-zero default'); + assert.equal(effective.shellProfile, '', 'an empty string must override a non-empty default'); +}); + +test('keys absent from the defaults are ignored', () => { + const effective = resolveEffectiveSettings(DEFAULTS, { notADefault: 'x' }, { alsoNot: 'y' }); + assert.equal('notADefault' in effective, false); + assert.equal('alsoNot' in effective, false); +}); + +test('the inputs are not mutated', () => { + const defaults = { permissionMode: null }; + const global = { permissionMode: 'plan' }; + const project = { permissionMode: null }; + resolveEffectiveSettings(defaults, global, project); + assert.deepEqual(defaults, { permissionMode: null }); + assert.deepEqual(global, { permissionMode: 'plan' }); + assert.deepEqual(project, { permissionMode: null }); +}); + +test('global and project default to empty when omitted', () => { + assert.deepEqual(resolveEffectiveSettings(DEFAULTS), DEFAULTS); +});