fix(settings): honor an explicitly saved null in get-effective-settings - #88
Merged
abasiri merged 2 commits intoSep 5, 2026
Merged
Conversation
get-effective-settings skipped a saved value when it was `null`, treating it as "never configured". But the settings panel persists permissionMode's "Default (none)" choice as `value || null`, so `null` is a deliberate value there meaning "pass no --permission-mode flag" — not an absent key. Two consequences: - A project that narrowed permissionMode back to "Default" kept inheriting the global mode. Set a global mode, then pick "Default" for one project, and the project still launches with the global mode. The narrower, more restrictive choice silently loses. - Any SETTING_DEFAULTS entry whose value is not null is unreachable at project scope, because a project-level null can never override it. Only `undefined` should fall through to the next-broader scope. Other falsy values (`false`, `0`, `''`) were already handled correctly and stay that way. Extracts the merge loop into resolve-effective-settings.js so the rule can be unit-tested without booting Electron, matching how folder-index-state.js and decodeOsc52Payload are already covered. Happy to inline it back into main.js if you'd rather not have the extra module — the behavioral change is the two dropped `!== null` clauses. 10 tests in test/resolve-effective-settings.test.js. Three of them fail if the `!== null` clauses are put back.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
An explicitly saved project Permission Mode of Default (none) is stored as
null. Previously the settings merge discarded it, so a project could inherit the globalbypassPermissionsmode despite selecting Default.Resolve settings in scope order (defaults, global, project), treating only
undefinedas absent. An explicitly savednulloverrides broader values. The resolver is used inside the current sharedeffectiveSettings(projectPath)helper, preserving its settings IPC and task-setup callers and the current Codex settings.Merged current
maininto this branch to resolve the integration conflict without rewriting the original contribution's history. The resulting PR remains scoped to the settings helper and its tests.Reproduction
Before: Switchboard supplies
--permission-mode bypassPermissions.After: Switchboard supplies no
--permission-modeflag. Claude's own configuration still applies; the separatedangerouslySkipPermissionsoption is unchanged.Validation
npm test: 172 passed, 0 failed on the updated branch (160 current-main tests plus 12 resolver/launch tests).node --check main.jsandgit diff --cached --checkpassed.NODE_PATH; no live Electron GUI or packaging test was performed locally. GitHub build checks run on the pushed commit.