Skip to content

feat(claude): add permission mode on the Agents settings page - #497

Open
Adam-Dalloul wants to merge 2 commits into
xintaofei:mainfrom
Adam-Dalloul:feat/claude-permission-mode
Open

feat(claude): add permission mode on the Agents settings page#497
Adam-Dalloul wants to merge 2 commits into
xintaofei:mainfrom
Adam-Dalloul:feat/claude-permission-mode

Conversation

@Adam-Dalloul

@Adam-Dalloul Adam-Dalloul commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Grok and Cursor already have a permission-mode control on Settings -> Agents. This adds the same kind of dropdown for Claude Code.

It writes the official permissions.defaultMode key and keeps existing allow/deny rules. It does not write skipDangerousModePermissionPrompt. That flag is Claude Code's own bypass-dialog consent, not a CodeG switch.

On save, a chosen mode is also written to CodeG's composer default (codeg:selector-prefs) so connect cannot keep a stickier last-used mode. Leaving Use default does not touch the composer last-used.

Labels match the composer: Manual / Accept Edits / Auto / Bypass Permissions. plan and dontAsk already on disk stay as-is instead of being shown as Use default.

New chats only. Extra custom Claude slots still use their isolated settings.json because they do not get this panel.

@xintaofei

Copy link
Copy Markdown
Owner

Thanks for putting this together — the mechanics here are genuinely well done, and I want to say that up front before I get into the two things I'd like to sort out.

What I verified as correct:

  • The key is real and so are the values. @agentclientprotocol/claude-agent-acp@0.69.0 (our pinned adapter) does read it — dist/acp-agent.js resolvePermissionMode(settingsManager.getSettings().permissions?.defaultMode) — and its alias table accepts auto and the manualdefault alias exactly as you implemented them.
  • The write target and the unset path are right. src-tauri/src/commands/acp.rs:7405 maps ClaudeCode~/.claude/settings.json, and because markRemovedKeysNull (acp-agent-settings.tsx:1410) recurses and merge_json_values (acp.rs:7459) deletes on null, clearing the nested permissions.defaultMode while keeping permissions.allow really does reach disk.
  • Nice catch wiring claudePermissionMode into handleConfigTextChange — editing the raw JSON re-syncs the dropdown, so there's no stale-display bug.
  • Checks are green on the branch: full suite 312 files / 4172 tests, tsc --noEmit clean, eslint clean, and all 10 locales have all 8 keys under AcpAgentSettings.claude.

So the plumbing is solid. The two issues are about how this interacts with machinery that already exists.


1. We already have a persistent default mode for Claude, and it silently wins over this one

This is the blocker. The composer's mode dropdown is already a sticky per-agent default for new chats, and it overrides permissions.defaultMode after the adapter has applied it:

  • src/lib/selector-prefs-storage.ts persists codeg:selector-prefs[<agentType>].modeId whenever the user picks a mode.
  • New/draft conversations seed the composer from getSavedModeId(agentType)conversation-detail-panel.tsx:323.
  • Every connect ships it: acp-connections-context.tsx:4802acpConnect(..., savedPrefs.modeId, ...).
  • The backend applies it after session creation: connection.rs:5836-5893 apply_preferred_session_options calls session/set_mode whenever the saved mode differs from the session's current mode — and that current mode is precisely what the adapter just derived from settings.json.
  • Reusing a live connection doesn't rescue it either: use-connection-lifecycle.ts:430 re-applies the composer's mode at send time.

So for a user with no saved preference, your setting works. For a user who has picked a mode in the composer even once, the adapter reads your value and then codeg immediately overwrites it — the Settings page and the composer end up showing different things with no explanation.

Two follow-on notes:

  • The PR description says the default "had to be edited by hand in ~/.claude/settings.json". That isn't quite the case — the composer dropdown was already a persisted default-for-new-chats. Worth re-framing, since it changes what problem this is solving.
  • The reason Grok legitimately has this control is that Grok exposes no ACP modes channel (documented at connection.rs:920), so its settings panel is the only control point. Claude does expose one, which is why the two collide here.

I'd also push back gently on "safe because it fails closed": the override lands on the saved value, not the more restrictive one. Settings = default with a saved bypassPermissions yields bypass — so an attempt to reduce permissions from this panel is exactly the case that gets silently defeated. That's the part that makes it blocking rather than cosmetic for me.

The fix isn't obvious and I don't want to hand you a design by fiat, but the shape is: pick one source of truth and make the other follow it. Either this panel writes through to the selector pref (and the composer reads it back), or connect stops sending preferred_mode_id for claude_code when permissions.defaultMode is set. Whichever way, it needs a stated precedence contract and a test covering Settings=default + saved=bypassPermissions and the inverse.

2. skipDangerousModePermissionPrompt — I don't think we should write this at all

acp-agent-settings.tsx:678 sets it on bypass and never clears it, and acp-agent-settings.test.tsx:1255-1261 locks that in.

That key isn't a behavior switch — the SDK types it as "Whether the user has accepted the bypass permissions mode dialog". It's a record of consent. Claude Code itself only writes it after the user actually accepts the dialog (tengu_bypass_permissions_mode_dialog_accept). Writing it from a settings dropdown records an acceptance the user never gave, in a file that also governs their terminal claude, claude --bg, and the claude agents dispatcher.

I initially thought this might be load-bearing for us — that bypass wouldn't work over ACP without it — so I dug in, and it isn't. The function that drops a requested bypassPermissions when consent is missing has exactly one call site in the bundled SDK binary, and it's the interactive claude agents background-dispatch view, not the SDK stream path. The separate downgrade helper is gated on CLAUDE_CODE_SESSION_KIND === "bg". Meanwhile our adapter passes permissionMode and allowDangerouslySkipPermissions explicitly (dist/acp-agent.js:4845). So bypass over ACP works without the flag, and the only thing the write accomplishes is suppressing a safety interlock outside codeg.

My suggestion is to just drop it. If we ever do decide codeg should offer to record that consent, it wants its own explicit, clearly-worded confirmation at save time — not a side effect of a dropdown whose hint text only mentions permissions.defaultMode. Auto-clearing on leaving bypass isn't a good fallback either, since the flag may well predate codeg and we'd have no way to know.

3. Smaller things (non-blocking)

  • plan and dontAsk are valid defaultMode values, so a user who already has one of them set will see the dropdown display "Use default" (normalizeClaudePermissionMode returns "" for both) — and touching the dropdown then silently replaces their setting. Either surface them as options or don't let the control overwrite a value it can't represent. Offering only four values matches the Grok panel, so I read the omission itself as deliberate; it's the misreporting I'd fix.
  • Label drift: the adapter's own composer labels are "Manual" / "Accept Edits" / "Auto" / "Bypass Permissions"; this panel calls the same ids "Ask every time" / "Auto-approve edits" / "Smart auto-approve" / "Always approve". Same modes, two vocabularies, one app.
  • auto is model-gated in the composer (supportsAutoMode) and bypass is suppressed when running as root, but Settings offers both unconditionally. The adapter clamps safely, so it's over-promising rather than broken.

None of this is a knock on the implementation quality — the pure function, the merge-delete behavior, and the i18n coverage are all clean, and it clearly wasn't guesswork. It's that the feature lands on top of an existing default-mode mechanism it doesn't yet know about. Sort out the precedence in #1 and drop the consent flag in #2, and I'd be happy to take this.

Happy to talk through the precedence design if it'd help — that's the part with real trade-offs.

Claude Code had no Settings toggle for the start-of-session permission
mode, unlike Grok and Cursor. Write the official
permissions.defaultMode key in ~/.claude/settings.json and skip the
bypass confirmation dialog when Always approve is selected.
Settings save writes the chosen mode into codeg:selector-prefs so
connect cannot keep a stickier last-used composer mode.

Do not stamp skipDangerousModePermissionPrompt. Preserve plan/dontAsk
on disk instead of showing Use default. Labels match the composer.
@Adam-Dalloul
Adam-Dalloul force-pushed the feat/claude-permission-mode branch from d9110c6 to 12e5c08 Compare August 20, 2026 19:40
@Adam-Dalloul

Copy link
Copy Markdown
Contributor Author

You were right about the two stores. I went with write-through: saving a chosen mode also updates codeg:selector-prefs, so Settings=default now beats a leftover bypass in the composer default. Use default still leaves the last-used composer value alone.

Dropped skipDangerousModePermissionPrompt. Labels now match the composer. plan / dontAsk on disk round-trip instead of looking like Use default.

Tests cover Settings=default vs saved bypass and the inverse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants