mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
Respect user settings for permission mode and model selection (#340)
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> --------- Co-authored-by: Tim Visher <194828183+timvisher-dd@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Codex <codex@openai.com>
This commit is contained in:
3
.prettierignore
Normal file
3
.prettierignore
Normal file
@@ -0,0 +1,3 @@
|
||||
node_modules
|
||||
dist
|
||||
.agent-shell
|
||||
123
src/acp-agent.ts
123
src/acp-agent.ts
@@ -194,6 +194,43 @@ function isStaticBinary(): boolean {
|
||||
const IS_ROOT = (process.geteuid?.() ?? process.getuid?.()) === 0;
|
||||
const ALLOW_BYPASS = !IS_ROOT || !!process.env.IS_SANDBOX;
|
||||
|
||||
const PERMISSION_MODE_ALIASES: Record<string, PermissionMode> = {
|
||||
default: "default",
|
||||
acceptedits: "acceptEdits",
|
||||
dontask: "dontAsk",
|
||||
plan: "plan",
|
||||
bypasspermissions: "bypassPermissions",
|
||||
bypass: "bypassPermissions",
|
||||
};
|
||||
|
||||
export function resolvePermissionMode(defaultMode?: unknown): PermissionMode {
|
||||
if (defaultMode === undefined) {
|
||||
return "default";
|
||||
}
|
||||
|
||||
if (typeof defaultMode !== "string") {
|
||||
throw new Error("Invalid permissions.defaultMode: expected a string.");
|
||||
}
|
||||
|
||||
const normalized = defaultMode.trim().toLowerCase();
|
||||
if (normalized === "") {
|
||||
throw new Error("Invalid permissions.defaultMode: expected a non-empty string.");
|
||||
}
|
||||
|
||||
const mapped = PERMISSION_MODE_ALIASES[normalized];
|
||||
if (!mapped) {
|
||||
throw new Error(`Invalid permissions.defaultMode: ${defaultMode}.`);
|
||||
}
|
||||
|
||||
if (mapped === "bypassPermissions" && !ALLOW_BYPASS) {
|
||||
throw new Error(
|
||||
"Invalid permissions.defaultMode: bypassPermissions is not available when running as root.",
|
||||
);
|
||||
}
|
||||
|
||||
return mapped;
|
||||
}
|
||||
|
||||
// Implement the ACP Agent interface
|
||||
export class ClaudeAcpAgent implements Agent {
|
||||
sessions: {
|
||||
@@ -1047,7 +1084,9 @@ export class ClaudeAcpAgent implements Agent {
|
||||
}
|
||||
}
|
||||
|
||||
const permissionMode = "default";
|
||||
const permissionMode = resolvePermissionMode(
|
||||
settingsManager.getSettings().permissions?.defaultMode,
|
||||
);
|
||||
|
||||
// Extract options from _meta if provided
|
||||
const userProvidedOptions = (params._meta as NewSessionMeta | undefined)?.claudeCode?.options;
|
||||
@@ -1259,6 +1298,79 @@ function buildConfigOptions(
|
||||
];
|
||||
}
|
||||
|
||||
// Claude Code CLI persists display strings like "opus[1m]" in settings,
|
||||
// but the SDK model list uses IDs like "claude-opus-4-6-1m".
|
||||
const MODEL_CONTEXT_HINT_PATTERN = /\[(\d+m)\]$/i;
|
||||
|
||||
function tokenizeModelPreference(model: string): { tokens: string[]; contextHint?: string } {
|
||||
const lower = model.trim().toLowerCase();
|
||||
const contextHint = lower.match(MODEL_CONTEXT_HINT_PATTERN)?.[1]?.toLowerCase();
|
||||
|
||||
const normalized = lower.replace(MODEL_CONTEXT_HINT_PATTERN, " $1 ");
|
||||
const rawTokens = normalized.split(/[^a-z0-9]+/).filter(Boolean);
|
||||
const tokens = rawTokens
|
||||
.map((token) => {
|
||||
if (token === "opusplan") return "opus";
|
||||
if (token === "best" || token === "default") return "";
|
||||
return token;
|
||||
})
|
||||
.filter((token) => token && token !== "claude")
|
||||
.filter((token) => /[a-z]/.test(token) || token.endsWith("m"));
|
||||
|
||||
return { tokens, contextHint };
|
||||
}
|
||||
|
||||
function scoreModelMatch(model: ModelInfo, tokens: string[], contextHint?: string): number {
|
||||
const haystack = `${model.value} ${model.displayName}`.toLowerCase();
|
||||
let score = 0;
|
||||
for (const token of tokens) {
|
||||
if (haystack.includes(token)) {
|
||||
score += token === contextHint ? 3 : 1;
|
||||
}
|
||||
}
|
||||
return score;
|
||||
}
|
||||
|
||||
function resolveModelPreference(models: ModelInfo[], preference: string): ModelInfo | null {
|
||||
const trimmed = preference.trim();
|
||||
if (!trimmed) return null;
|
||||
|
||||
const lower = trimmed.toLowerCase();
|
||||
|
||||
// Exact match on value or display name
|
||||
const directMatch = models.find(
|
||||
(model) =>
|
||||
model.value === trimmed ||
|
||||
model.value.toLowerCase() === lower ||
|
||||
model.displayName.toLowerCase() === lower,
|
||||
);
|
||||
if (directMatch) return directMatch;
|
||||
|
||||
// Substring match
|
||||
const includesMatch = models.find((model) => {
|
||||
const value = model.value.toLowerCase();
|
||||
const display = model.displayName.toLowerCase();
|
||||
return value.includes(lower) || display.includes(lower) || lower.includes(value);
|
||||
});
|
||||
if (includesMatch) return includesMatch;
|
||||
|
||||
// Tokenized matching for aliases like "opus[1m]"
|
||||
const { tokens, contextHint } = tokenizeModelPreference(trimmed);
|
||||
if (tokens.length === 0) return null;
|
||||
|
||||
let bestMatch: ModelInfo | null = null;
|
||||
let bestScore = 0;
|
||||
for (const model of models) {
|
||||
const score = scoreModelMatch(model, tokens, contextHint);
|
||||
if (0 < score && (!bestMatch || bestScore < score)) {
|
||||
bestMatch = model;
|
||||
bestScore = score;
|
||||
}
|
||||
}
|
||||
|
||||
return bestMatch;
|
||||
}
|
||||
|
||||
async function getAvailableModels(
|
||||
query: Query,
|
||||
models: ModelInfo[],
|
||||
@@ -1269,14 +1381,7 @@ async function getAvailableModels(
|
||||
let currentModel = models[0];
|
||||
|
||||
if (settings.model) {
|
||||
const match = models.find(
|
||||
(m) =>
|
||||
m.value === settings.model ||
|
||||
m.value.includes(settings.model!) ||
|
||||
settings.model!.includes(m.value) ||
|
||||
m.displayName.toLowerCase() === settings.model!.toLowerCase() ||
|
||||
m.displayName.toLowerCase().includes(settings.model!.toLowerCase()),
|
||||
);
|
||||
const match = resolveModelPreference(models, settings.model);
|
||||
if (match) {
|
||||
currentModel = match;
|
||||
}
|
||||
|
||||
@@ -14,7 +14,12 @@ import { CLAUDE_CONFIG_DIR } from "./acp-agent.js";
|
||||
* Docs: https://code.claude.com/docs/en/iam#tool-specific-permission-rules
|
||||
*/
|
||||
|
||||
export interface PermissionSettings {
|
||||
defaultMode?: string;
|
||||
}
|
||||
|
||||
export interface ClaudeCodeSettings {
|
||||
permissions?: PermissionSettings;
|
||||
env?: Record<string, string>;
|
||||
model?: string;
|
||||
}
|
||||
@@ -162,6 +167,13 @@ export class SettingsManager {
|
||||
if (settings.model) {
|
||||
merged.model = settings.model;
|
||||
}
|
||||
|
||||
if (settings.permissions?.defaultMode !== undefined) {
|
||||
merged.permissions = {
|
||||
...merged.permissions,
|
||||
defaultMode: settings.permissions.defaultMode,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
this.mergedSettings = merged;
|
||||
|
||||
220
src/tests/acp-agent-settings.test.ts
Normal file
220
src/tests/acp-agent-settings.test.ts
Normal file
@@ -0,0 +1,220 @@
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import * as fs from "node:fs";
|
||||
import * as path from "node:path";
|
||||
import * as os from "node:os";
|
||||
import type { AgentSideConnection } from "@agentclientprotocol/sdk";
|
||||
import type { ClaudeAcpAgent as ClaudeAcpAgentType } from "../acp-agent.js";
|
||||
|
||||
const { querySpy } = vi.hoisted(() => ({
|
||||
querySpy: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@anthropic-ai/claude-agent-sdk", async () => {
|
||||
const actual = await vi.importActual<any>("@anthropic-ai/claude-agent-sdk");
|
||||
return {
|
||||
...actual,
|
||||
query: querySpy,
|
||||
};
|
||||
});
|
||||
|
||||
describe("ClaudeAcpAgent settings", () => {
|
||||
let tempDir: string;
|
||||
let originalClaudeConfigDir: string | undefined;
|
||||
|
||||
function createMockClient(): AgentSideConnection {
|
||||
return {
|
||||
sessionUpdate: async () => {},
|
||||
requestPermission: async () => ({ outcome: { outcome: "cancelled" } }),
|
||||
readTextFile: async () => ({ content: "" }),
|
||||
writeTextFile: async () => ({}),
|
||||
} as unknown as AgentSideConnection;
|
||||
}
|
||||
|
||||
function mockQuery() {
|
||||
let capturedOptions: any;
|
||||
const setModelSpy = vi.fn();
|
||||
querySpy.mockImplementation(({ options }: any) => {
|
||||
capturedOptions = options;
|
||||
return {
|
||||
initializationResult: async () => ({
|
||||
models: [
|
||||
{
|
||||
value: "claude-sonnet-4-5",
|
||||
displayName: "Claude Sonnet 4.5",
|
||||
description: "Default",
|
||||
},
|
||||
],
|
||||
}),
|
||||
setModel: setModelSpy,
|
||||
supportedCommands: async () => [],
|
||||
} as any;
|
||||
});
|
||||
return { getCapturedOptions: () => capturedOptions, setModelSpy };
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "acp-agent-settings-"));
|
||||
originalClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR;
|
||||
process.env.CLAUDE_CONFIG_DIR = tempDir;
|
||||
querySpy.mockReset();
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
if (originalClaudeConfigDir) {
|
||||
process.env.CLAUDE_CONFIG_DIR = originalClaudeConfigDir;
|
||||
} else {
|
||||
delete process.env.CLAUDE_CONFIG_DIR;
|
||||
}
|
||||
await fs.promises.rm(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("uses permissions.defaultMode for new sessions", async () => {
|
||||
await fs.promises.writeFile(
|
||||
path.join(tempDir, "settings.json"),
|
||||
JSON.stringify({
|
||||
permissions: {
|
||||
defaultMode: "dontAsk",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const projectDir = path.join(tempDir, "project");
|
||||
await fs.promises.mkdir(projectDir, { recursive: true });
|
||||
|
||||
const { getCapturedOptions } = mockQuery();
|
||||
|
||||
const { ClaudeAcpAgent } = await import("../acp-agent.js");
|
||||
const agent: ClaudeAcpAgentType = new ClaudeAcpAgent(createMockClient());
|
||||
|
||||
const response = await (agent as any).createSession({
|
||||
cwd: projectDir,
|
||||
mcpServers: [],
|
||||
_meta: { disableBuiltInTools: true },
|
||||
});
|
||||
|
||||
expect(getCapturedOptions().permissionMode).toBe("dontAsk");
|
||||
expect(getCapturedOptions().settingSources).toEqual(["user", "project", "local"]);
|
||||
expect(response.modes.currentModeId).toBe("dontAsk");
|
||||
});
|
||||
|
||||
it("supports acceptEdits mode defaults", async () => {
|
||||
await fs.promises.writeFile(
|
||||
path.join(tempDir, "settings.json"),
|
||||
JSON.stringify({
|
||||
permissions: {
|
||||
defaultMode: "acceptEdits",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const projectDir = path.join(tempDir, "project");
|
||||
await fs.promises.mkdir(projectDir, { recursive: true });
|
||||
|
||||
const { getCapturedOptions } = mockQuery();
|
||||
|
||||
const { ClaudeAcpAgent } = await import("../acp-agent.js");
|
||||
const agent: ClaudeAcpAgentType = new ClaudeAcpAgent(createMockClient());
|
||||
|
||||
const response = await (agent as any).createSession({
|
||||
cwd: projectDir,
|
||||
mcpServers: [],
|
||||
_meta: { disableBuiltInTools: true },
|
||||
});
|
||||
|
||||
expect(getCapturedOptions().permissionMode).toBe("acceptEdits");
|
||||
expect(response.modes.currentModeId).toBe("acceptEdits");
|
||||
});
|
||||
|
||||
it("defaults to 'default' when no permissions.defaultMode is set", async () => {
|
||||
const projectDir = path.join(tempDir, "project");
|
||||
await fs.promises.mkdir(projectDir, { recursive: true });
|
||||
|
||||
const { getCapturedOptions } = mockQuery();
|
||||
|
||||
const { ClaudeAcpAgent } = await import("../acp-agent.js");
|
||||
const agent: ClaudeAcpAgentType = new ClaudeAcpAgent(createMockClient());
|
||||
|
||||
const response = await (agent as any).createSession({
|
||||
cwd: projectDir,
|
||||
mcpServers: [],
|
||||
_meta: { disableBuiltInTools: true },
|
||||
});
|
||||
|
||||
expect(getCapturedOptions().permissionMode).toBe("default");
|
||||
expect(response.modes.currentModeId).toBe("default");
|
||||
});
|
||||
|
||||
it("throws when permissions.defaultMode is not a string", async () => {
|
||||
await fs.promises.writeFile(
|
||||
path.join(tempDir, "settings.json"),
|
||||
JSON.stringify({
|
||||
permissions: {
|
||||
defaultMode: 123,
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const projectDir = path.join(tempDir, "project");
|
||||
await fs.promises.mkdir(projectDir, { recursive: true });
|
||||
|
||||
mockQuery();
|
||||
|
||||
const { ClaudeAcpAgent } = await import("../acp-agent.js");
|
||||
const agent: ClaudeAcpAgentType = new ClaudeAcpAgent(createMockClient());
|
||||
|
||||
await expect(
|
||||
(agent as any).createSession({
|
||||
cwd: projectDir,
|
||||
mcpServers: [],
|
||||
_meta: { disableBuiltInTools: true },
|
||||
}),
|
||||
).rejects.toThrow("Invalid permissions.defaultMode");
|
||||
});
|
||||
|
||||
it("resolves model aliases like opus[1m] to the correct model", async () => {
|
||||
await fs.promises.writeFile(
|
||||
path.join(tempDir, "settings.json"),
|
||||
JSON.stringify({
|
||||
model: "opus[1m]",
|
||||
}),
|
||||
);
|
||||
|
||||
const projectDir = path.join(tempDir, "project");
|
||||
await fs.promises.mkdir(projectDir, { recursive: true });
|
||||
|
||||
const setModelSpy = vi.fn();
|
||||
querySpy.mockImplementation(({ options: _options }: any) => {
|
||||
return {
|
||||
initializationResult: async () => ({
|
||||
models: [
|
||||
{
|
||||
value: "claude-opus-4-6",
|
||||
displayName: "Claude Opus 4.6",
|
||||
description: "Base",
|
||||
},
|
||||
{
|
||||
value: "claude-opus-4-6-1m",
|
||||
displayName: "Claude Opus 4.6 (1M)",
|
||||
description: "Long context",
|
||||
},
|
||||
],
|
||||
}),
|
||||
setModel: setModelSpy,
|
||||
supportedCommands: async () => [],
|
||||
} as any;
|
||||
});
|
||||
|
||||
const { ClaudeAcpAgent } = await import("../acp-agent.js");
|
||||
const agent: ClaudeAcpAgentType = new ClaudeAcpAgent(createMockClient());
|
||||
|
||||
const response = await (agent as any).createSession({
|
||||
cwd: projectDir,
|
||||
mcpServers: [],
|
||||
_meta: { disableBuiltInTools: true },
|
||||
});
|
||||
|
||||
expect(setModelSpy).toHaveBeenCalledWith("claude-opus-4-6-1m");
|
||||
expect(response.models.currentModelId).toBe("claude-opus-4-6-1m");
|
||||
});
|
||||
});
|
||||
43
src/tests/resolve-permission-mode.test.ts
Normal file
43
src/tests/resolve-permission-mode.test.ts
Normal file
@@ -0,0 +1,43 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { resolvePermissionMode } from "../acp-agent.js";
|
||||
|
||||
describe("resolvePermissionMode", () => {
|
||||
it("returns 'default' when no mode is provided", () => {
|
||||
expect(resolvePermissionMode()).toBe("default");
|
||||
expect(resolvePermissionMode(undefined)).toBe("default");
|
||||
});
|
||||
|
||||
it("resolves exact canonical modes", () => {
|
||||
expect(resolvePermissionMode("default")).toBe("default");
|
||||
expect(resolvePermissionMode("acceptEdits")).toBe("acceptEdits");
|
||||
expect(resolvePermissionMode("dontAsk")).toBe("dontAsk");
|
||||
expect(resolvePermissionMode("plan")).toBe("plan");
|
||||
expect(resolvePermissionMode("bypassPermissions")).toBe("bypassPermissions");
|
||||
});
|
||||
|
||||
it("resolves case-insensitive aliases", () => {
|
||||
expect(resolvePermissionMode("DontAsk")).toBe("dontAsk");
|
||||
expect(resolvePermissionMode("DONTASK")).toBe("dontAsk");
|
||||
expect(resolvePermissionMode("AcceptEdits")).toBe("acceptEdits");
|
||||
expect(resolvePermissionMode("bypass")).toBe("bypassPermissions");
|
||||
});
|
||||
|
||||
it("trims whitespace", () => {
|
||||
expect(resolvePermissionMode(" dontAsk ")).toBe("dontAsk");
|
||||
});
|
||||
|
||||
it("throws on non-string values", () => {
|
||||
expect(() => resolvePermissionMode(123)).toThrow("expected a string");
|
||||
expect(() => resolvePermissionMode(true)).toThrow("expected a string");
|
||||
expect(() => resolvePermissionMode({})).toThrow("expected a string");
|
||||
});
|
||||
|
||||
it("throws on empty string", () => {
|
||||
expect(() => resolvePermissionMode("")).toThrow("expected a non-empty string");
|
||||
expect(() => resolvePermissionMode(" ")).toThrow("expected a non-empty string");
|
||||
});
|
||||
|
||||
it("throws on unknown mode", () => {
|
||||
expect(() => resolvePermissionMode("yolo")).toThrow("Invalid permissions.defaultMode: yolo");
|
||||
});
|
||||
});
|
||||
@@ -52,5 +52,42 @@ describe("SettingsManager", () => {
|
||||
settings = settingsManager.getSettings();
|
||||
expect(settings.model).toBe("claude-3-5-haiku");
|
||||
});
|
||||
|
||||
it("should merge permissions.defaultMode with later sources taking precedence", async () => {
|
||||
const claudeDir = path.join(tempDir, ".claude");
|
||||
await fs.promises.mkdir(claudeDir, { recursive: true });
|
||||
|
||||
await fs.promises.writeFile(
|
||||
path.join(claudeDir, "settings.json"),
|
||||
JSON.stringify({
|
||||
permissions: {
|
||||
defaultMode: "dontAsk",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
settingsManager = new SettingsManager(tempDir);
|
||||
await settingsManager.initialize();
|
||||
|
||||
let settings = settingsManager.getSettings();
|
||||
expect(settings.permissions?.defaultMode).toBe("dontAsk");
|
||||
|
||||
// Local settings override the mode
|
||||
await fs.promises.writeFile(
|
||||
path.join(claudeDir, "settings.local.json"),
|
||||
JSON.stringify({
|
||||
permissions: {
|
||||
defaultMode: "plan",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
settingsManager.dispose();
|
||||
settingsManager = new SettingsManager(tempDir);
|
||||
await settingsManager.initialize();
|
||||
|
||||
settings = settingsManager.getSettings();
|
||||
expect(settings.permissions?.defaultMode).toBe("plan");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user