Skip to content

Respect user settings for permission mode and model selection#340

Merged
benbrandt merged 6 commits intozed-industries:mainfrom
timvisher-dd:respect-model_-permissions_-and-other-settings-from-all-claude-locations
Mar 3, 2026
Merged

Respect user settings for permission mode and model selection#340
benbrandt merged 6 commits intozed-industries:mainfrom
timvisher-dd:respect-model_-permissions_-and-other-settings-from-all-claude-locations

Conversation

@timvisher-dd
Copy link
Contributor

@timvisher-dd timvisher-dd commented Feb 22, 2026

Most user settings (env, permission rules like allow/deny/ask) already work through the SDK's settingSources or are handled by the CLI subprocess. Two settings require explicit handling by the ACP agent and were not being respected:

permissions.defaultMode: Must be passed as the permissionMode option to query(). The SDK defaults to "default" internally and does not read it from settings files. The CLI handles this by reading the setting and passing it to the SDK; createSession was hardcoding it to "default" instead.

model: The CLI persists display strings like "opus[1m]" in settings, but the SDK model list uses IDs like "claude-opus-4-6-1m". The existing substring matching in getAvailableModels couldn't resolve these aliases, so they silently fell back to the first model. Passing the raw alias to query.setModel() doesn't work either — the CLI's internal dg() function strips [1m] and loses the context hint.

Changes:

  • Fix pre-existing build errors from SDK type changes (SDKTaskStartedMessage removed, SDKMessageTemp replaced with SDK's own SDKMessage)
  • Add permissions.defaultMode to ClaudeCodeSettings, merge it through the settings hierarchy, and resolve it via resolvePermissionMode() with case-insensitive aliases, validation, and root-user guard
  • Add tokenized model matching (resolveModelPreference) that handles CLI display strings like "opus[1m]" by scoring against the available models list
  • Add unit and integration tests for both features

Co-authored-by: Codex codex@openai.com

@cla-bot cla-bot bot added the cla-signed label Feb 22, 2026
@timvisher-dd timvisher-dd force-pushed the respect-model_-permissions_-and-other-settings-from-all-claude-locations branch from 4d82563 to 1060ce2 Compare February 22, 2026 21:31
@timvisher-dd timvisher-dd changed the title Respect permissions.defaultMode from settings Respect user settings for permission mode and model selection Feb 22, 2026
@timvisher-dd timvisher-dd marked this pull request as ready for review February 22, 2026 22:06
@timvisher-dd
Copy link
Contributor Author

I'm not sure why but CI doesn't seem to be kicking off. I'll do my best to keep an eye open for failures there if/when it runs.

import { randomUUID } from "node:crypto";
import { fileURLToPath } from "node:url";

// SDK has an unresolved rate limit type, reconstructing with the rest for now
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi I assume this might have been unintentional... there is actually an issue in the SDK where they have an unresolved type that makes all SDKMessages resolve to any with typescript, which is why this is here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I'm sorry about that I missed it in my review. I'll deal with this post-haste.

Copy link
Member

@benbrandt benbrandt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we are just doing our best to find a model match without a way to do so in the SDK, I think these are mostly fine, but would appreciate if some of the unrelated changes were removed

@timvisher-dd timvisher-dd force-pushed the respect-model_-permissions_-and-other-settings-from-all-claude-locations branch from 0337553 to c501213 Compare February 23, 2026 12:45
@timvisher-dd
Copy link
Contributor Author

Given that we are just doing our best to find a model match without a way to do so in the SDK, I think these are mostly fine, but would appreciate if some of the unrelated changes were removed

How's that grab your feathers?

Copy link
Member

@benbrandt benbrandt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@timvisher-dd
Copy link
Contributor Author

thanks!

🚀 I'll try to fix the CI failures.

@timvisher-dd timvisher-dd force-pushed the respect-model_-permissions_-and-other-settings-from-all-claude-locations branch from c501213 to ccfc88d Compare February 27, 2026 00:50
@timvisher-dd
Copy link
Contributor Author

thanks!

🚀 I'll try to fix the CI failures.

@benbrandt I think you'll need to approve the CI run again? I don't want to bother squashing the commits together until I see CI passing. I also did not fix the TypeScript build warnings again.

@timvisher-dd timvisher-dd marked this pull request as draft February 28, 2026 20:34
timvisher-dd and others added 4 commits February 28, 2026 16:31
Honor Claude's built-in settings hierarchy for the initial permission
mode instead of hardcoding it to "default".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Codex <codex@openai.com>
The CLI persists display strings like "opus[1m]" in settings files,
but the SDK model list uses IDs like "claude-opus-4-6-1m". Add
tokenized model matching that resolves these aliases against the
available models list so the correct model ID is passed to
query.setModel() and reported as currentModelId to ACP clients.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Codex <codex@openai.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prefix destructured `options` with `_` to satisfy
@typescript-eslint/no-unused-vars rule.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@timvisher-dd timvisher-dd force-pushed the respect-model_-permissions_-and-other-settings-from-all-claude-locations branch from ccfc88d to bae9ed6 Compare February 28, 2026 21:33
@timvisher-dd
Copy link
Contributor Author

@benbrandt Build please? 🙏 🙏 🙏 🙇‍♂️

timvisher-dd and others added 2 commits March 1, 2026 13:20
Prettier was failing on .agent-shell/transcripts/ markdown files
that aren't part of the project source.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@timvisher-dd
Copy link
Contributor Author

@benbrandt Build please. :) 🙏 🙏 🙏 🙇‍♂️

@benbrandt benbrandt marked this pull request as ready for review March 3, 2026 11:54
@benbrandt benbrandt merged commit 6253b16 into zed-industries:main Mar 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants