Skip to content

Commit dadaa44

Browse files
fix(settings): honor explicitly saved null settings (#88)
Resolve defaults, global settings, and project settings through the shared effectiveSettings helper, allowing explicit null values to override inherited settings. Preserve current Codex and task-shell settings. Validated with 172 passing local tests and successful macOS, Windows, Linux x64, and Linux arm64 builds.
1 parent 7dc0701 commit dadaa44

3 files changed

Lines changed: 175 additions & 10 deletions

File tree

‎main.js‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ const { shouldStartFresh } = require('./session-launch');
3434
const { discoverShellProfiles, getShellProfiles, resolveShell, isWindows, isWslShell, windowsToWslPath, shellArgs, quoteArgvForShell } = require('./shell-profiles');
3535
const { startScheduler } = require('./schedule-runner');
3636
const { encodeProjectPath } = require('./encode-project-path');
37+
const { resolveEffectiveSettings } = require('./resolve-effective-settings');
3738
const { listProjectDirectory, readProjectFile } = require('./project-files');
3839
const { createTaskManager } = require('./task-manager');
3940

@@ -974,16 +975,7 @@ ipcMain.handle('get-shell-profiles', () => {
974975
function effectiveSettings(projectPath) {
975976
const global = getSetting('global') || {};
976977
const project = projectPath ? (getSetting('project:' + projectPath) || {}) : {};
977-
const effective = { ...SETTING_DEFAULTS };
978-
for (const key of Object.keys(SETTING_DEFAULTS)) {
979-
if (global[key] !== undefined && global[key] !== null) {
980-
effective[key] = global[key];
981-
}
982-
if (project[key] !== undefined && project[key] !== null) {
983-
effective[key] = project[key];
984-
}
985-
}
986-
return effective;
978+
return resolveEffectiveSettings(SETTING_DEFAULTS, global, project);
987979
}
988980

989981
ipcMain.handle('get-effective-settings', (_event, projectPath) => effectiveSettings(projectPath));

‎resolve-effective-settings.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* Merge saved settings over defaults for a project.
3+
*
4+
* Scopes apply narrowest-last: defaults, then global, then project.
5+
*
6+
* Only `undefined` — the key was never saved at that scope — falls through to
7+
* the next-broader value. An explicit `null` is a real, deliberate choice and
8+
* wins like any other value. The settings panel relies on this: it persists
9+
* permissionMode's "Default (none)" option as `value || null`, so `null` there
10+
* means "pass no --permission-mode flag", not "unset".
11+
*
12+
* Used by main.js's shared effectiveSettings helper so the settings IPC and
13+
* task setup resolve the same values, without needing Electron in unit tests.
14+
*/
15+
function resolveEffectiveSettings(defaults, global = {}, project = {}) {
16+
const effective = { ...defaults };
17+
for (const key of Object.keys(defaults)) {
18+
if (global[key] !== undefined) {
19+
effective[key] = global[key];
20+
}
21+
if (project[key] !== undefined) {
22+
effective[key] = project[key];
23+
}
24+
}
25+
return effective;
26+
}
27+
28+
module.exports = { resolveEffectiveSettings };
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
const test = require('node:test');
2+
const assert = require('node:assert/strict');
3+
4+
const { resolveEffectiveSettings } = require('../resolve-effective-settings');
5+
const claude = require('../harnesses/claude');
6+
const codex = require('../harnesses/codex');
7+
8+
// Mirrors the shape of main.js's SETTING_DEFAULTS for the keys that matter here.
9+
const DEFAULTS = {
10+
permissionMode: null,
11+
dangerouslySkipPermissions: false,
12+
worktree: false,
13+
visibleSessionCount: 5,
14+
shellProfile: 'auto',
15+
};
16+
17+
test('returns the defaults when nothing has been saved', () => {
18+
assert.deepEqual(resolveEffectiveSettings(DEFAULTS, {}, {}), DEFAULTS);
19+
});
20+
21+
test('project scope overrides global, which overrides defaults', () => {
22+
const effective = resolveEffectiveSettings(
23+
DEFAULTS,
24+
{ permissionMode: 'plan', visibleSessionCount: 10 },
25+
{ permissionMode: 'acceptEdits' },
26+
);
27+
assert.equal(effective.permissionMode, 'acceptEdits', 'project wins over global');
28+
assert.equal(effective.visibleSessionCount, 10, 'global still applies where project is silent');
29+
assert.equal(effective.shellProfile, 'auto', 'untouched keys keep their default');
30+
});
31+
32+
test('a project can narrow permissionMode back to Default over a global mode', () => {
33+
// The settings panel saves the "Default (none)" option as `value || null`, so
34+
// an explicit null means "pass no --permission-mode flag". Claude's own
35+
// configuration still applies. It must beat a broader-scope mode.
36+
const effective = resolveEffectiveSettings(
37+
DEFAULTS,
38+
{ permissionMode: 'bypassPermissions' },
39+
{ permissionMode: null },
40+
);
41+
assert.equal(effective.permissionMode, null,
42+
'an explicitly saved null must not fall back to the global mode');
43+
});
44+
45+
test('an explicit global null beats a non-null default', () => {
46+
// With a null default this is invisible, so pin it against a default that is
47+
// not null — otherwise any future non-null SETTING_DEFAULTS value silently
48+
// becomes unreachable.
49+
const effective = resolveEffectiveSettings(
50+
{ permissionMode: 'acceptEdits' },
51+
{ permissionMode: null },
52+
{},
53+
);
54+
assert.equal(effective.permissionMode, null,
55+
'an explicitly saved null must override a non-null default');
56+
});
57+
58+
test('undefined means "never saved" and falls through', () => {
59+
const effective = resolveEffectiveSettings(
60+
DEFAULTS,
61+
{ permissionMode: 'plan' },
62+
{ permissionMode: undefined },
63+
);
64+
assert.equal(effective.permissionMode, 'plan',
65+
'an absent project key must not shadow the global value');
66+
});
67+
68+
test('null and undefined are not conflated', () => {
69+
const withNull = resolveEffectiveSettings(DEFAULTS, { permissionMode: 'plan' }, { permissionMode: null });
70+
const withUndefined = resolveEffectiveSettings(DEFAULTS, { permissionMode: 'plan' }, {});
71+
assert.notEqual(withNull.permissionMode, withUndefined.permissionMode,
72+
'an explicit null and an absent key must resolve differently');
73+
});
74+
75+
test('other falsy values are preserved', () => {
76+
const effective = resolveEffectiveSettings(
77+
{ worktree: true, visibleSessionCount: 5, shellProfile: 'auto' },
78+
{},
79+
{ worktree: false, visibleSessionCount: 0, shellProfile: '' },
80+
);
81+
assert.equal(effective.worktree, false, 'false must override a true default');
82+
assert.equal(effective.visibleSessionCount, 0, '0 must override a non-zero default');
83+
assert.equal(effective.shellProfile, '', 'an empty string must override a non-empty default');
84+
});
85+
86+
test('keys absent from the defaults are ignored', () => {
87+
const effective = resolveEffectiveSettings(DEFAULTS, { notADefault: 'x' }, { alsoNot: 'y' });
88+
assert.equal('notADefault' in effective, false);
89+
assert.equal('alsoNot' in effective, false);
90+
});
91+
92+
test('the inputs are not mutated', () => {
93+
const defaults = { permissionMode: null };
94+
const global = { permissionMode: 'plan' };
95+
const project = { permissionMode: null };
96+
resolveEffectiveSettings(defaults, global, project);
97+
assert.deepEqual(defaults, { permissionMode: null });
98+
assert.deepEqual(global, { permissionMode: 'plan' });
99+
assert.deepEqual(project, { permissionMode: null });
100+
});
101+
102+
test('global and project default to empty when omitted', () => {
103+
assert.deepEqual(resolveEffectiveSettings(DEFAULTS), DEFAULTS);
104+
});
105+
106+
test('project Default removes the inherited Claude permission flag on launch and resume', () => {
107+
const global = { permissionMode: 'bypassPermissions' };
108+
for (const isNew of [true, false]) {
109+
const inherited = claude.buildLaunchArgs({
110+
sessionId: 'session', isNew,
111+
options: resolveEffectiveSettings(DEFAULTS, global),
112+
});
113+
assert.ok(inherited.includes('--permission-mode'));
114+
assert.ok(inherited.includes('bypassPermissions'));
115+
116+
const overridden = claude.buildLaunchArgs({
117+
sessionId: 'session', isNew,
118+
options: resolveEffectiveSettings(DEFAULTS, global, { permissionMode: null }),
119+
});
120+
assert.ok(!overridden.includes('--permission-mode'));
121+
assert.ok(!overridden.includes('bypassPermissions'));
122+
assert.ok(!overridden.includes('--dangerously-skip-permissions'));
123+
}
124+
});
125+
126+
test('Claude Default preserves independently configured Codex and task shell settings', () => {
127+
const options = resolveEffectiveSettings(
128+
{ ...DEFAULTS, codexSandbox: '', codexApproval: '', codexModel: '' },
129+
{
130+
permissionMode: 'bypassPermissions',
131+
codexSandbox: 'workspace-write',
132+
codexApproval: 'never',
133+
shellProfile: 'custom-shell',
134+
},
135+
{ permissionMode: null, codexSandbox: 'read-only', codexApproval: 'on-request' },
136+
);
137+
const args = codex.buildLaunchArgs({ sessionId: 'session', isNew: true, options });
138+
assert.equal(options.permissionMode, null);
139+
assert.equal(options.shellProfile, 'custom-shell');
140+
assert.ok(args.includes('read-only'));
141+
assert.ok(args.includes('on-request'));
142+
assert.ok(!args.includes('workspace-write'));
143+
assert.ok(!args.includes('never'));
144+
assert.ok(!args.includes('--permission-mode'));
145+
});

0 commit comments

Comments
 (0)