From c9bcfa7638a8d3fb4b29784938d3c0f04e84f79b Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sat, 18 Jul 2026 23:03:39 +0200 Subject: [PATCH] Use safer automatic approval modes by default (#2213) * feat(providers): default to safer automatic approvals * fix(providers): preserve defaults on older Codex versions * test(providers): cover Claude automatic approval default * fix(providers): preserve Claude defaults on cloud transports * fix(providers): resolve defaults from launch capabilities --- .../resolve-agent-form.test.ts | 19 ++++++ .../provider-selection/resolve-agent-form.ts | 7 ++- packages/protocol/src/provider-manifest.ts | 4 +- .../src/server/agent/agent-manager.test.ts | 59 +++++++++++++++---- .../server/src/server/agent/agent-manager.ts | 32 +++++++--- .../src/server/agent/agent-sdk-types.ts | 7 +++ .../server/agent/provider-registry.test.ts | 27 ++++++++- .../src/server/agent/provider-registry.ts | 19 +++++- .../agent/provider-snapshot-manager.test.ts | 32 +++++++++- .../server/agent/provider-snapshot-manager.ts | 2 + .../providers/claude/agent.redesign.test.ts | 31 ++++++++++ .../server/agent/providers/claude/agent.ts | 24 +++++++- .../agent/providers/codex-app-server-agent.ts | 17 +++++- 13 files changed, 253 insertions(+), 27 deletions(-) diff --git a/packages/app/src/provider-selection/resolve-agent-form.test.ts b/packages/app/src/provider-selection/resolve-agent-form.test.ts index 992c3a8f0..a10de7b12 100644 --- a/packages/app/src/provider-selection/resolve-agent-form.test.ts +++ b/packages/app/src/provider-selection/resolve-agent-form.test.ts @@ -575,6 +575,25 @@ describe("resolveFormState", () => { expect(resolved.modeId).toBe("workspace-write"); }); + it("falls back when the provider cannot advertise its preferred default mode", () => { + const providerMap = makeProviderMap({ + ...TEST_CODEX_DEFINITION, + defaultModeId: "auto-review", + modes: TEST_CODEX_DEFINITION.modes, + }); + + const resolved = resolveFormState( + undefined, + { provider: "codex" }, + CODEX_MODELS, + INITIAL_USER_MODIFIED, + makeState({ provider: "codex" }).form, + providerMap, + ); + + expect(resolved.modeId).toBe("auto"); + }); + it("ignores disabled ready providers when resolving selectable defaults", () => { const entries: ProviderSnapshotEntry[] = [ { diff --git a/packages/app/src/provider-selection/resolve-agent-form.ts b/packages/app/src/provider-selection/resolve-agent-form.ts index 196b16bc2..56e84f5b1 100644 --- a/packages/app/src/provider-selection/resolve-agent-form.ts +++ b/packages/app/src/provider-selection/resolve-agent-form.ts @@ -171,7 +171,12 @@ function resolvePreferredModeId(input: { const preferredModeId = normalizeSelectedModeId(input.preferredModeId); if (preferredModeId) return preferredModeId; - return input.providerDef?.defaultModeId ?? input.providerDef?.modes[0]?.id ?? ""; + const defaultModeId = input.providerDef?.defaultModeId; + const modes = input.providerDef?.modes ?? []; + if (defaultModeId && (modes.length === 0 || modes.some((mode) => mode.id === defaultModeId))) { + return defaultModeId; + } + return modes[0]?.id ?? ""; } export function mergeSelectedComposerPreferences(args: { diff --git a/packages/protocol/src/provider-manifest.ts b/packages/protocol/src/provider-manifest.ts index 37c79e59a..b3e388faa 100644 --- a/packages/protocol/src/provider-manifest.ts +++ b/packages/protocol/src/provider-manifest.ts @@ -185,7 +185,7 @@ export const AGENT_PROVIDER_DEFINITIONS: AgentProviderDefinition[] = [ id: "claude", label: "Claude", description: "Anthropic's multi-tool assistant with MCP support, streaming, and deep reasoning", - defaultModeId: "default", + defaultModeId: "auto", modes: CLAUDE_MODES, voice: { enabled: true, @@ -197,7 +197,7 @@ export const AGENT_PROVIDER_DEFINITIONS: AgentProviderDefinition[] = [ id: "codex", label: "Codex", description: "OpenAI's Codex workspace agent with sandbox controls and optional network access", - defaultModeId: "auto", + defaultModeId: "auto-review", modes: CODEX_MODES, voice: { enabled: true, diff --git a/packages/server/src/server/agent/agent-manager.test.ts b/packages/server/src/server/agent/agent-manager.test.ts index 161a2b416..e5f20da37 100644 --- a/packages/server/src/server/agent/agent-manager.test.ts +++ b/packages/server/src/server/agent/agent-manager.test.ts @@ -35,6 +35,7 @@ import type { AgentStreamEvent, AgentTimelineItem, ImportProviderSessionInput, + ResolveAgentDefaultModeInput, } from "./agent-sdk-types.js"; import type { PaseoToolCatalog } from "./tools/types.js"; import type { ProviderDefinition } from "./provider-registry.js"; @@ -111,11 +112,15 @@ function expectArchivedAgentRecord( } class TestAgentClient implements AgentClient { - readonly provider = "codex" as const; + readonly provider: AgentProvider; readonly capabilities = TEST_CAPABILITIES; readonly createdConfigs: AgentSessionConfig[] = []; readonly resumeOverrides: Array | undefined> = []; + constructor(provider: AgentProvider = "codex") { + this.provider = provider; + } + async isAvailable(): Promise { return true; } @@ -129,18 +134,18 @@ class TestAgentClient implements AgentClient { return { models: [ { - provider: "codex", + provider: this.provider, id: "gpt-5.4", label: "GPT-5.4", isDefault: true, }, { - provider: "codex", + provider: this.provider, id: "gpt-5.4-mini", label: "GPT-5.4 Mini", }, { - provider: "codex", + provider: this.provider, id: "gpt-5.2-codex", label: "GPT-5.2 Codex", }, @@ -156,7 +161,7 @@ class TestAgentClient implements AgentClient { ): Promise { this.resumeOverrides.push(config); return new TestAgentSession({ - provider: "codex", + provider: this.provider, cwd: config?.cwd ?? process.cwd(), daemonAppendSystemPrompt: config?.daemonAppendSystemPrompt, }); @@ -935,9 +940,43 @@ test("normalizeConfig injects the provider default model when omitted", async () ); expect(snapshot.config.model).toBe("gpt-5.4"); + expect(snapshot.config.modeId).toBe("auto-review"); +}); + +test("normalizeConfig injects Claude's automatic approval default when omitted", async () => { + const workdir = mkdtempSync(join(tmpdir(), "agent-manager-claude-default-test-")); + const manager = new AgentManager({ + clients: { claude: new TestAgentClient("claude") }, + logger, + }); + + const snapshot = await manager.createAgent({ provider: "claude", cwd: workdir }, undefined, { + workspaceId: undefined, + }); + expect(snapshot.config.modeId).toBe("auto"); }); +test("normalizeConfig uses a capability-aware provider mode default", async () => { + const workdir = mkdtempSync(join(tmpdir(), "agent-manager-mode-default-test-")); + class CapabilityAwareClient extends TestAgentClient { + override async resolveDefaultModeId(input: ResolveAgentDefaultModeInput): Promise { + return input.env?.CLAUDE_CODE_USE_BEDROCK === "1" ? "default" : "auto"; + } + } + const manager = new AgentManager({ + clients: { codex: new CapabilityAwareClient() }, + logger, + }); + + const snapshot = await manager.createAgent({ provider: "codex", cwd: workdir }, undefined, { + workspaceId: undefined, + env: { CLAUDE_CODE_USE_BEDROCK: "1" }, + }); + + expect(snapshot.config.modeId).toBe("default"); +}); + test("createAgent forwards request env into the spawned provider process", async () => { const workdir = mkdtempSync(join(tmpdir(), "agent-manager-env-test-")); const client = new EnvProbeAgentClient(); @@ -997,7 +1036,7 @@ test("normalizeConfig strips legacy 'default' model id", async () => { ); expect(snapshot.config.model).toBe("gpt-5.4"); - expect(snapshot.config.modeId).toBe("auto"); + expect(snapshot.config.modeId).toBe("auto-review"); }); test("listDraftCommands returns no commands without guessing a missing model", async () => { @@ -1094,7 +1133,7 @@ test("listDraftCommands uses explicit model config without default model fetchin provider: "codex", cwd: workdir, model: "gpt-5.4", - modeId: "auto", + modeId: "auto-review", }, ]); }); @@ -1194,7 +1233,7 @@ test("listDraftFeatures uses explicit model config without default model fetchin provider: "codex", cwd: workdir, model: "gpt-5.4", - modeId: "auto", + modeId: "auto-review", }, ]); }); @@ -1651,7 +1690,7 @@ test("createAgent passes daemon launch env through the provider launch context", provider: "codex", cwd: workdir, model: "gpt-5.4", - modeId: "auto", + modeId: "auto-review", }); expect(client.lastLaunchContext).toEqual({ agentId: snapshot.id, @@ -2448,7 +2487,7 @@ test("resumeAgentFromPersistence keeps metadata config, applies overrides, and p }); expect(client.lastResumeOverrides).toMatchObject({ model: "gpt-5.4", - modeId: "auto", + modeId: "auto-review", systemPrompt: "new prompt", mcpServers: { paseo: { diff --git a/packages/server/src/server/agent/agent-manager.ts b/packages/server/src/server/agent/agent-manager.ts index 6f0eb9cc0..0bcbc2ca8 100644 --- a/packages/server/src/server/agent/agent-manager.ts +++ b/packages/server/src/server/agent/agent-manager.ts @@ -117,6 +117,7 @@ interface PreparedSessionConfig { interface NormalizeConfigOptions { resolveDefaultModel?: boolean; + env?: Record; } interface TimeoutOptions { @@ -1034,7 +1035,11 @@ export class AgentManager { this.assertAcceptingAgentRegistrations(); const resolvedAgentId = validateAgentId(agentId ?? this.idFactory(), "createAgent"); await this.deleteAgentState(resolvedAgentId); - const { storedConfig, launchConfig } = await this.prepareSessionConfig(config, resolvedAgentId); + const { storedConfig, launchConfig } = await this.prepareSessionConfig( + config, + resolvedAgentId, + options?.env, + ); this.requireEnabledProvider(storedConfig.provider); const client = await this.requireAvailableClient({ provider: storedConfig.provider, @@ -4075,17 +4080,27 @@ export class AgentManager { } if (!normalized.modeId) { - try { - normalized.modeId = - getAgentProviderDefinition(normalized.provider).defaultModeId ?? undefined; - } catch { - // Unknown provider - } + normalized.modeId = await this.resolveDefaultModeId(normalized, options.env); } return normalized; } + private async resolveDefaultModeId( + config: AgentSessionConfig, + env?: Record, + ): Promise { + const providerDefault = await this.clients + .get(config.provider) + ?.resolveDefaultModeId?.({ config, env }); + if (providerDefault) return providerDefault; + try { + return getAgentProviderDefinition(config.provider).defaultModeId ?? undefined; + } catch { + return undefined; + } + } + private async resolveDefaultModelId(config: AgentSessionConfig): Promise { const client = this.clients.get(config.provider); if (!client) { @@ -4107,8 +4122,9 @@ export class AgentManager { private async prepareSessionConfig( config: AgentSessionConfig, agentId: string, + env?: Record, ): Promise { - const storedConfig = await this.normalizeConfig(stripInternalPaseoMcpServer(config)); + const storedConfig = await this.normalizeConfig(stripInternalPaseoMcpServer(config), { env }); const launchConfig = this.applyDaemonAppendSystemPrompt( withRuntimePaseoMcpServer({ config: storedConfig, diff --git a/packages/server/src/server/agent/agent-sdk-types.ts b/packages/server/src/server/agent/agent-sdk-types.ts index 3813e4eec..c5b8e37b1 100644 --- a/packages/server/src/server/agent/agent-sdk-types.ts +++ b/packages/server/src/server/agent/agent-sdk-types.ts @@ -668,6 +668,12 @@ export type FetchCatalogOptions = export interface ProviderCatalog { models: AgentModelDefinition[]; modes: AgentMode[]; + defaultModeId?: string | null; +} + +export interface ResolveAgentDefaultModeInput { + config: AgentSessionConfig; + env?: Record; } export interface AgentClient { @@ -690,6 +696,7 @@ export interface AgentClient { * The registry is responsible for merging configured model overrides. */ fetchCatalog(options: FetchCatalogOptions): Promise; + resolveDefaultModeId?(input: ResolveAgentDefaultModeInput): Promise; resolveCreateConfig?(input: ResolveAgentCreateConfigInput): ResolveAgentCreateConfigResult; isCreateConfigUnattended?(input: AgentCreateConfigUnattendedInput): boolean; listCommands?(config: AgentSessionConfig): Promise; diff --git a/packages/server/src/server/agent/provider-registry.test.ts b/packages/server/src/server/agent/provider-registry.test.ts index 177c55477..4596b6d91 100644 --- a/packages/server/src/server/agent/provider-registry.test.ts +++ b/packages/server/src/server/agent/provider-registry.test.ts @@ -814,7 +814,7 @@ test("enabled: false keeps provider metadata in registry", () => { id: "claude", label: "Claude", description: "Anthropic's multi-tool assistant with MCP support, streaming, and deep reasoning", - defaultModeId: "default", + defaultModeId: "auto", enabled: false, }); expect(registry.claude.modes).toEqual( @@ -1487,6 +1487,31 @@ describe("fetchCatalog", () => { expect(catalog.models.map((model) => model.id)).toEqual(["profile-model", "extra-model"]); }); + test("replacement models still resolve the provider's capability-aware default mode", async () => { + const resolveDefaultModeId = vi.fn(async () => "default"); + const injectedClient = { + provider: "codex", + capabilities: {}, + resolveDefaultModeId, + isAvailable: vi.fn(async () => true), + } satisfies Partial as AgentClient; + const registry = buildProviderRegistry(logger, { + providerOverrides: { + codex: { models: [{ id: "profile-model", label: "Profile Model" }] }, + }, + }); + + const catalog = await registry.codex.fetchCatalog( + { scope: "workspace", cwd: "/tmp/catalog", force: false }, + injectedClient, + ); + + expect(catalog.defaultModeId).toBe("default"); + expect(resolveDefaultModeId).toHaveBeenCalledWith({ + config: { provider: "codex", cwd: "/tmp/catalog" }, + }); + }); + test("additionalModels can override replacement model fields", async () => { const registry = buildProviderRegistry(logger, { providerOverrides: { diff --git a/packages/server/src/server/agent/provider-registry.ts b/packages/server/src/server/agent/provider-registry.ts index 9d9022f5a..dbdb5c076 100644 --- a/packages/server/src/server/agent/provider-registry.ts +++ b/packages/server/src/server/agent/provider-registry.ts @@ -14,6 +14,7 @@ import type { ProviderCatalog, ResolveAgentCreateConfigInput, ResolveAgentCreateConfigResult, + ResolveAgentDefaultModeInput, } from "./agent-sdk-types.js"; import { isDefaultAgentCreateConfigUnattended, @@ -427,12 +428,20 @@ function wrapClientProvider( fetchCatalog: async (options) => { const catalog = await inner.fetchCatalog(options); return { + ...catalog, models: mergeModels(provider, profileModels, additionalModels, catalog.models, { profileModelsAreAdditive, }), modes: catalog.modes, }; }, + resolveDefaultModeId: inner.resolveDefaultModeId + ? async ({ config, env }: ResolveAgentDefaultModeInput) => + await inner.resolveDefaultModeId?.({ + config: { ...config, provider: inner.provider }, + env, + }) + : undefined, resolveCreateConfig: inner.resolveCreateConfig?.bind(inner), isCreateConfigUnattended: inner.isCreateConfigUnattended?.bind(inner), listFeatures: listFeatures @@ -516,17 +525,25 @@ function createRegistryEntry( // the single catalog API; otherwise use static/empty modes with no runtime. const models = mergeModelAdditions(provider, replacementModels, resolved.additionalModels); if (hasStaticModes) { + const defaultModeId = await catalogClient.resolveDefaultModeId?.({ + config: { + provider, + cwd: options.scope === "workspace" ? options.cwd : process.cwd(), + }, + }); return { models, modes: decorateModes(resolved.definition.modes), + defaultModeId, }; } const catalog = await catalogClient.fetchCatalog(options); - return { models, modes: decorateModes(catalog.modes) }; + return { ...catalog, models, modes: decorateModes(catalog.modes) }; } const catalog = await catalogClient.fetchCatalog(options); return { + ...catalog, models: mergeModels( provider, resolved.profileModels, diff --git a/packages/server/src/server/agent/provider-snapshot-manager.test.ts b/packages/server/src/server/agent/provider-snapshot-manager.test.ts index 4b6a3eac5..980abe088 100644 --- a/packages/server/src/server/agent/provider-snapshot-manager.test.ts +++ b/packages/server/src/server/agent/provider-snapshot-manager.test.ts @@ -117,9 +117,11 @@ describe("ProviderSnapshotManager public surface", () => { try { const snapshot = manager.getSnapshot("/tmp/project"); const claude = snapshot.find((entry) => entry.provider === "claude"); + const codex = snapshot.find((entry) => entry.provider === "codex"); expect(claude?.status).toBe("loading"); expect(claude?.label).toBe("Claude"); - expect(claude?.defaultModeId).toBe("default"); + expect(claude?.defaultModeId).toBe("auto"); + expect(codex?.defaultModeId).toBe("auto-review"); } finally { manager.destroy(); } @@ -220,6 +222,34 @@ describe("ProviderSnapshotManager public surface", () => { } }); + test("ready snapshots publish the catalog's capability-aware default mode", async () => { + const manager = new ProviderSnapshotManager({ + logger: createTestLogger(), + extraClients: { + codex: createExtraClient("codex", { + isAvailable: async () => true, + fetchCatalog: async () => ({ + models: [], + modes: [{ id: "default", label: "Default", description: "Ask before running tools" }], + defaultModeId: "default", + }), + }), + }, + }); + + try { + const entry = await manager.getProvider({ + cwd: "/tmp/project", + provider: "codex", + wait: true, + }); + + expect(entry).toMatchObject({ status: "ready", defaultModeId: "default" }); + } finally { + manager.destroy(); + } + }); + test("explicit refresh re-probes only the requested warm provider", async () => { const cwd = "/tmp/project"; const isAvailableCodex = vi.fn(async () => true); diff --git a/packages/server/src/server/agent/provider-snapshot-manager.ts b/packages/server/src/server/agent/provider-snapshot-manager.ts index f21d91923..771c4d24c 100644 --- a/packages/server/src/server/agent/provider-snapshot-manager.ts +++ b/packages/server/src/server/agent/provider-snapshot-manager.ts @@ -799,6 +799,8 @@ export class ProviderSnapshotManager { setEntry({ ...base, + defaultModeId: + catalog.defaultModeId === undefined ? definition.defaultModeId : catalog.defaultModeId, status: "ready", enabled: true, models: catalog.models, diff --git a/packages/server/src/server/agent/providers/claude/agent.redesign.test.ts b/packages/server/src/server/agent/providers/claude/agent.redesign.test.ts index f43bbacf7..1af10b8a5 100644 --- a/packages/server/src/server/agent/providers/claude/agent.redesign.test.ts +++ b/packages/server/src/server/agent/providers/claude/agent.redesign.test.ts @@ -218,6 +218,37 @@ test("rejects auto mode when Claude Code uses Bedrock", async () => { } }); +test.each([ + { env: {}, expected: "auto" }, + { env: { CLAUDE_CODE_USE_BEDROCK: "1" }, expected: "default" }, + { env: { CLAUDE_CODE_USE_VERTEX: "true" }, expected: "default" }, +])("defaults to $expected for transport environment $env", async ({ env, expected }) => { + const client = new ClaudeAgentClient({ + logger: createTestLogger(), + runtimeSettings: { env }, + }); + + await expect( + client.resolveDefaultModeId({ + config: { provider: "claude", cwd: process.cwd() }, + }), + ).resolves.toBe(expected); +}); + +test("launch environment participates in the Claude default mode", async () => { + const client = new ClaudeAgentClient({ + logger: createTestLogger(), + runtimeSettings: { env: {} }, + }); + + await expect( + client.resolveDefaultModeId({ + config: { provider: "claude", cwd: process.cwd() }, + env: { CLAUDE_CODE_USE_BEDROCK: "1" }, + }), + ).resolves.toBe("default"); +}); + test("allows launch env to disable inherited Bedrock transport for auto mode", async () => { const previousBedrock = process.env.CLAUDE_CODE_USE_BEDROCK; process.env.CLAUDE_CODE_USE_BEDROCK = "1"; diff --git a/packages/server/src/server/agent/providers/claude/agent.ts b/packages/server/src/server/agent/providers/claude/agent.ts index a0403f2e6..4a07fd4fe 100644 --- a/packages/server/src/server/agent/providers/claude/agent.ts +++ b/packages/server/src/server/agent/providers/claude/agent.ts @@ -91,6 +91,7 @@ import { type ListImportableSessionsOptions, type McpServerConfig, type ProviderCatalog, + type ResolveAgentDefaultModeInput, } from "../../agent-sdk-types.js"; import { importSessionFromPersistence } from "../../provider-session-import.js"; import { @@ -1472,7 +1473,28 @@ export class ClaudeAgentClient implements AgentClient { async fetchCatalog(_options: FetchCatalogOptions): Promise { // Claude exposes a global catalog here; cwd/force are intentionally irrelevant. const models = await getClaudeModelsWithSettings(this.logger, this.configDir); - return { models, modes: DEFAULT_MODES }; + const modes = detectIneligibleAutoModeTransport( + createProviderEnv({ baseEnv: process.env, runtimeSettings: this.runtimeSettings }), + ) + ? DEFAULT_MODES.filter((mode) => mode.id !== "auto") + : DEFAULT_MODES; + return { + models, + modes, + defaultModeId: modes.some((mode) => mode.id === "auto") ? "auto" : "default", + }; + } + + async resolveDefaultModeId({ + config, + env: launchEnv, + }: ResolveAgentDefaultModeInput): Promise { + const env = createProviderEnv({ + baseEnv: process.env, + runtimeSettings: this.runtimeSettings, + overlays: [config.extra?.claude?.env, launchEnv], + }); + return detectIneligibleAutoModeTransport(env) ? "default" : "auto"; } async listFeatures(config: AgentSessionConfig): Promise { diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index f816e33b0..b1d12194c 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -6379,8 +6379,21 @@ export class CodexAppServerAgentClient implements AgentClient { } async fetchCatalog(_options: FetchCatalogOptions): Promise { - const models = await this.fetchModelsFromAppServer(); - return { models, modes: CODEX_MODES }; + const [models, autoReviewEnabled] = await Promise.all([ + this.fetchModelsFromAppServer(), + this.resolveAutoReviewEnabled(), + ]); + return { + models, + defaultModeId: autoReviewEnabled ? "auto-review" : DEFAULT_CODEX_MODE_ID, + modes: autoReviewEnabled + ? CODEX_MODES + : CODEX_MODES.filter((mode) => mode.id !== "auto-review"), + }; + } + + async resolveDefaultModeId(): Promise { + return (await this.resolveAutoReviewEnabled()) ? "auto-review" : DEFAULT_CODEX_MODE_ID; } private async fetchModelsFromAppServer(): Promise {