feat(permissions): default unconfigured installs to the auto permission mode - #90
Merged
Merged
Conversation
…on mode Narrowed successor to #87. The PERMISSION_MODES hoist that #87 also carried landed upstream independently as a7698f4 and arrived via the v0.0.31 sync, so all that remains here is the main.js half, which upstream does not have. Two changes: - SETTING_DEFAULTS.permissionMode becomes 'auto' (was null), so a never-configured install gets Claude Code's own default mode instead of prompting for every action. - get-effective-settings stops skipping on an explicit `null`. Only `undefined` (key never saved at that scope) now falls through. The DB already distinguished the two — settings-panel.js persists the "Default" choice as `value || null`, i.e. a real null — but the read side collapsed them, so a user who deliberately picked "Default" was indistinguishable from one who never opened settings. Without this, flipping the default above would silently upgrade every explicit "Default" to 'auto'. The second change is a bug in its own right, independent of the default flip: because an explicit null never won, a project that narrowed permissionMode back to "Default" kept inheriting the global mode, and any SETTING_DEFAULTS value other than null was unreachable at project scope. It is being contributed upstream separately, without the default flip. test/dom-permission-mode-picker.test.js covers both halves: the pickers offering `auto` (upstream's a7698f4, which shipped untested) and this branch's main.js semantics, including a guard that the `!== null` skip can't come back. 380/380 tests pass; eslint 0 errors.
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
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.
Narrowed successor to #87. Stacked on #88 (the v0.0.31 upstream sync) — based on
chore/sync-upstream-v0.0.31and targeting it, so the diff stays clean. GitHub will retarget this tomainonce #88 merges.Why this is smaller than #87
#87 did two things: hoist the thrice-duplicated permission-mode list into a
PERMISSION_MODESconstant, and changemain.js's default resolution. Upstream implemented the identical hoist independently ina7698f4— same diagnosis, same solution, same three call sites — and it arrived via #88. That half is no longer ours to contribute, so it is dropped here. #87 is closed as superseded.What upstream does not have is the
main.jshalf, which is all that's left:Changes
1.
SETTING_DEFAULTS.permissionMode: nullto'auto'— a never-configured install gets Claude Code's own default mode rather than prompting for every action.2.
get-effective-settingsno longer skips on an explicitnull. Onlyundefined(key never saved at that scope) falls through now.The second is a prerequisite for the first, and also a bug on its own.
settings-panel.jspersists the "Default (none)" choice asvalue || null— a real, deliberatenull. The read side then treatednullthe same as "absent", so:permissionModeback to "Default" kept inheriting the global mode, andSETTING_DEFAULTSvalue other thannullwas unreachable at project scope.Without fixing that, flipping the default in change 1 would have silently upgraded every explicitly-chosen "Default" to
'auto'— turning a user's "prompt me for everything" into "let Claude classify". The DB already encoded the distinction; only the read side was wrong.That bug is independent of the default flip and is being contributed upstream separately, without the flip (which is this fork's preference, not an upstream concern).
Tests
test/dom-permission-mode-picker.test.js(8 tests) covers both halves, with the ownership split written down in its header:autoand staying driven by one shared list — upstream'sa7698f4, which shipped with no tests, so these guard against a future sync dropping itmain.jssemantics, including an explicit guard that the[key] !== nullskip cannot come backVerified the mode list against the real CLI: the 2.1.220 binary's
--permission-modechoice list isdefault,auto,acceptEdits,plan,dontAsk,bypassPermissions(plusmanual, which upstream deliberately omits as UI-confusing next to "Default" — that decision is kept).380/380 tests pass; eslint 0 errors.
Supersedes #89, which GitHub auto-closed when its base branch
chore/sync-upstream-v0.0.31was deleted on merge of #88. Same branch, same commit, retargeted tomain.