diff --git a/main.js b/main.js index 2fd68841..f19ff9f7 100644 --- a/main.js +++ b/main.js @@ -34,6 +34,7 @@ const { shouldStartFresh } = require('./session-launch'); 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'); const { listProjectDirectory, readProjectFile } = require('./project-files'); const { createTaskManager } = require('./task-manager'); @@ -974,16 +975,7 @@ ipcMain.handle('get-shell-profiles', () => { function effectiveSettings(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); } ipcMain.handle('get-effective-settings', (_event, projectPath) => effectiveSettings(projectPath)); diff --git a/resolve-effective-settings.js b/resolve-effective-settings.js new file mode 100644 index 00000000..917d8de9 --- /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". + * + * Used by main.js's shared effectiveSettings helper so the settings IPC and + * task setup resolve the same values, without needing Electron in unit tests. + */ +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..721d5d64 --- /dev/null +++ b/test/resolve-effective-settings.test.js @@ -0,0 +1,145 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { resolveEffectiveSettings } = require('../resolve-effective-settings'); +const claude = require('../harnesses/claude'); +const codex = require('../harnesses/codex'); + +// 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 means "pass no --permission-mode flag". Claude's own + // configuration still applies. 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); +}); + +test('project Default removes the inherited Claude permission flag on launch and resume', () => { + const global = { permissionMode: 'bypassPermissions' }; + for (const isNew of [true, false]) { + const inherited = claude.buildLaunchArgs({ + sessionId: 'session', isNew, + options: resolveEffectiveSettings(DEFAULTS, global), + }); + assert.ok(inherited.includes('--permission-mode')); + assert.ok(inherited.includes('bypassPermissions')); + + const overridden = claude.buildLaunchArgs({ + sessionId: 'session', isNew, + options: resolveEffectiveSettings(DEFAULTS, global, { permissionMode: null }), + }); + assert.ok(!overridden.includes('--permission-mode')); + assert.ok(!overridden.includes('bypassPermissions')); + assert.ok(!overridden.includes('--dangerously-skip-permissions')); + } +}); + +test('Claude Default preserves independently configured Codex and task shell settings', () => { + const options = resolveEffectiveSettings( + { ...DEFAULTS, codexSandbox: '', codexApproval: '', codexModel: '' }, + { + permissionMode: 'bypassPermissions', + codexSandbox: 'workspace-write', + codexApproval: 'never', + shellProfile: 'custom-shell', + }, + { permissionMode: null, codexSandbox: 'read-only', codexApproval: 'on-request' }, + ); + const args = codex.buildLaunchArgs({ sessionId: 'session', isNew: true, options }); + assert.equal(options.permissionMode, null); + assert.equal(options.shellProfile, 'custom-shell'); + assert.ok(args.includes('read-only')); + assert.ok(args.includes('on-request')); + assert.ok(!args.includes('workspace-write')); + assert.ok(!args.includes('never')); + assert.ok(!args.includes('--permission-mode')); +});