From 64b9b612230bb7fce840aa699ff82078a407c8ef Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 12 Apr 2026 21:37:59 +0700 Subject: [PATCH] Align MCP/CLI naming and resolve default model/mode server-side (#291) * Align create_agent naming and resolve defaults server-side * Fix test assertions for default model/mode resolution - Use config.model and config.modeId instead of non-existent snapshot.model - Strip legacy "default" model ID in normalizeConfig - Update runtimeInfo assertion to expect resolved default model * Fix CI: update CLI test for --title, fix formatting in checkout-git --- packages/cli/src/commands/agent/run.ts | 11 +++++--- packages/cli/tests/05-agent-run.test.ts | 2 +- packages/server/src/client/daemon-client.ts | 5 ---- .../src/server/agent/agent-management-mcp.ts | 21 ++++++++------ .../src/server/agent/agent-manager.test.ts | 28 ++++++++++++++++--- .../server/src/server/agent/agent-manager.ts | 28 +++++++++++++++++-- .../src/server/agent/agent-mcp.e2e.test.ts | 16 +++++------ .../src/server/agent/mcp-parity.e2e.test.ts | 10 +++---- .../src/server/agent/mcp-server.test.ts | 14 +++++----- .../server/src/server/agent/mcp-server.ts | 22 +++++++-------- packages/server/src/utils/checkout-git.ts | 8 +++--- 11 files changed, 105 insertions(+), 60 deletions(-) diff --git a/packages/cli/src/commands/agent/run.ts b/packages/cli/src/commands/agent/run.ts index c4817076b..321a1b9da 100644 --- a/packages/cli/src/commands/agent/run.ts +++ b/packages/cli/src/commands/agent/run.ts @@ -1,4 +1,4 @@ -import { Command } from "commander"; +import { Command, Option } from "commander"; import { getStructuredAgentResponse, StructuredAgentResponseError, @@ -22,7 +22,8 @@ export function addRunOptions(cmd: Command): Command { .description("Create and start an agent with a task") .argument("", "The task/prompt for the agent") .option("-d, --detach", "Run in background (detached)") - .option("--name ", "Assign a name/title to the agent") + .option("--title ", "Assign a title to the agent") + .addOption(new Option("--name <name>", "Hidden alias for --title").hideHelp()) .option( "--provider <provider>", "Agent provider, or provider/model (e.g. codex or codex/gpt-5.4)", @@ -82,6 +83,7 @@ export const agentRunSchema: OutputSchema<AgentRunResult> = { export interface AgentRunOptions extends CommandOptions { detach?: boolean; + title?: string; name?: string; provider?: string; model?: string; @@ -325,6 +327,7 @@ export async function runRunCommand( } const resolvedProviderModel = resolveProviderAndModel(options); + const resolvedTitle = options.title ?? options.name; let client; try { @@ -414,7 +417,7 @@ export async function runRunCommand( structuredAgent = await client.createAgent({ provider: resolvedProviderModel.provider, cwd, - title: options.name, + title: resolvedTitle, modeId: options.mode, model: resolvedProviderModel.model, thinkingOptionId, @@ -513,7 +516,7 @@ export async function runRunCommand( const agent = await client.createAgent({ provider: resolvedProviderModel.provider, cwd, - title: options.name, + title: resolvedTitle, modeId: options.mode, model: resolvedProviderModel.model, thinkingOptionId, diff --git a/packages/cli/tests/05-agent-run.test.ts b/packages/cli/tests/05-agent-run.test.ts index 5761f3290..6757f959b 100644 --- a/packages/cli/tests/05-agent-run.test.ts +++ b/packages/cli/tests/05-agent-run.test.ts @@ -54,7 +54,7 @@ try { assert.strictEqual(result.exitCode, 0, "run --help should exit 0"); assert(result.stdout.includes("-d"), "help should mention -d flag"); assert(result.stdout.includes("--detach"), "help should mention --detach flag"); - assert(result.stdout.includes("--name"), "help should mention --name option"); + assert(result.stdout.includes("--title"), "help should mention --title option"); assert(result.stdout.includes("--provider"), "help should mention --provider option"); assert(result.stdout.includes("--mode"), "help should mention --mode option"); assert(result.stdout.includes("--cwd"), "help should mention --cwd option"); diff --git a/packages/server/src/client/daemon-client.ts b/packages/server/src/client/daemon-client.ts index 68935f1f1..2d7ba7db4 100644 --- a/packages/server/src/client/daemon-client.ts +++ b/packages/server/src/client/daemon-client.ts @@ -71,7 +71,6 @@ import type { AgentSessionConfig, } from "../server/agent/agent-sdk-types.js"; import type { MutableDaemonConfig, MutableDaemonConfigPatch } from "../shared/messages.js"; -import { getAgentProviderDefinition } from "../server/agent/provider-manifest.js"; import { isRelayClientWebSocketUrl } from "../shared/daemon-endpoints.js"; import { asUint8Array, @@ -3938,10 +3937,6 @@ function resolveAgentConfig(options: CreateAgentRequestOptions): AgentSessionCon throw new Error("createAgent requires provider and cwd"); } - if (!merged.modeId) { - merged.modeId = getAgentProviderDefinition(merged.provider).defaultModeId ?? undefined; - } - return { ...merged, provider: merged.provider, diff --git a/packages/server/src/server/agent/agent-management-mcp.ts b/packages/server/src/server/agent/agent-management-mcp.ts index 1c44437a3..9ada46dec 100644 --- a/packages/server/src/server/agent/agent-management-mcp.ts +++ b/packages/server/src/server/agent/agent-management-mcp.ts @@ -104,7 +104,7 @@ export async function createAgentManagementMcpServer( .min(1, "Title is required") .max(60, "Title must be 60 characters or fewer") .describe("Short descriptive title (<= 60 chars) summarizing the agent's focus."), - agentType: AgentProviderEnum.optional().describe( + provider: AgentProviderEnum.optional().describe( "Optional agent implementation to spawn. Defaults to 'claude'.", ), model: z.string().optional().describe("Model to use (e.g. claude-sonnet-4-20250514)"), @@ -114,7 +114,10 @@ export async function createAgentManagementMcpServer( .string() .optional() .describe("Optional task to start immediately after creation (non-blocking)."), - initialMode: z.string().describe("Required session mode to configure before the first run."), + mode: z + .string() + .optional() + .describe("Optional session mode to configure before the first run."), worktreeName: z .string() .optional() @@ -159,9 +162,9 @@ export async function createAgentManagementMcpServer( async (args) => { const { cwd, - agentType, + provider, initialPrompt, - initialMode, + mode, worktreeName, baseBranch, background = false, @@ -171,9 +174,9 @@ export async function createAgentManagementMcpServer( labels, } = args as { cwd: string; - agentType?: AgentProvider; + provider?: AgentProvider; initialPrompt?: string; - initialMode: string; + mode?: string; worktreeName?: string; baseBranch?: string; background?: boolean; @@ -201,13 +204,13 @@ export async function createAgentManagementMcpServer( worktreeConfig = worktree; } - const provider: AgentProvider = agentType ?? "claude"; + const resolvedProvider: AgentProvider = provider ?? "claude"; const normalizedTitle = title?.trim() ?? null; const snapshot = await agentManager.createAgent( { - provider, + provider: resolvedProvider, cwd: resolvedCwd, - modeId: initialMode, + modeId: mode, title: normalizedTitle ?? undefined, model, thinkingOptionId: thinking, diff --git a/packages/server/src/server/agent/agent-manager.test.ts b/packages/server/src/server/agent/agent-manager.test.ts index 15fd92da9..2a104e09d 100644 --- a/packages/server/src/server/agent/agent-manager.test.ts +++ b/packages/server/src/server/agent/agent-manager.test.ts @@ -98,6 +98,22 @@ class TestAgentClient implements AgentClient { return new TestAgentSession(config); } + async listModels() { + return [ + { + provider: "codex", + id: "gpt-5.4", + label: "GPT-5.4", + isDefault: true, + }, + { + provider: "codex", + id: "gpt-5.4-mini", + label: "GPT-5.4 Mini", + }, + ]; + } + async resumeSession( _handle: AgentPersistenceHandle, config?: Partial<AgentSessionConfig>, @@ -212,7 +228,7 @@ function createFeature(overrides: Partial<AgentFeature> = {}): AgentFeature { describe("AgentManager", () => { const logger = createTestLogger(); - test("normalizeConfig does not inject default model when omitted", async () => { + test("normalizeConfig injects the provider default model when omitted", async () => { const workdir = mkdtempSync(join(tmpdir(), "agent-manager-test-")); const storagePath = join(workdir, "agents"); const storage = new AgentStorage(storagePath, logger); @@ -230,7 +246,8 @@ describe("AgentManager", () => { cwd: workdir, }); - expect(snapshot.model).toBeUndefined(); + expect(snapshot.config.model).toBe("gpt-5.4"); + expect(snapshot.config.modeId).toBe("auto"); }); test("normalizeConfig strips legacy 'default' model id", async () => { @@ -252,7 +269,8 @@ describe("AgentManager", () => { model: "default", }); - expect(snapshot.model).toBeUndefined(); + expect(snapshot.config.model).toBe("gpt-5.4"); + expect(snapshot.config.modeId).toBe("auto"); }); test("createAgent passes daemon launch env through the provider launch context", async () => { @@ -292,6 +310,8 @@ describe("AgentManager", () => { expect(client.lastConfig).toEqual({ provider: "codex", cwd: workdir, + model: "gpt-5.4", + modeId: "auto", }); expect(client.lastLaunchContext).toEqual({ env: { @@ -1432,7 +1452,7 @@ describe("AgentManager", () => { cwd: workdir, }); - expect(snapshot.runtimeInfo?.model ?? null).toBeNull(); + expect(snapshot.runtimeInfo?.model).toBe("gpt-5.4"); await manager.runAgent(snapshot.id, "hello"); diff --git a/packages/server/src/server/agent/agent-manager.ts b/packages/server/src/server/agent/agent-manager.ts index e9b8f309a..014459b98 100644 --- a/packages/server/src/server/agent/agent-manager.ts +++ b/packages/server/src/server/agent/agent-manager.ts @@ -33,7 +33,7 @@ import type { PersistedAgentDescriptor, } from "./agent-sdk-types.js"; import type { AgentStorage } from "./agent-storage.js"; -import { AGENT_PROVIDER_IDS } from "./provider-manifest.js"; +import { AGENT_PROVIDER_IDS, getAgentProviderDefinition } from "./provider-manifest.js"; export { AGENT_LIFECYCLE_STATUSES, type AgentLifecycleStatus }; @@ -2568,7 +2568,31 @@ export class AgentManager { if (typeof normalized.model === "string") { const trimmed = normalized.model.trim(); - normalized.model = trimmed.length > 0 ? trimmed : undefined; + normalized.model = trimmed.length > 0 && trimmed !== "default" ? trimmed : undefined; + } + + if (!normalized.model) { + const client = this.clients.get(normalized.provider); + if (client) { + try { + const models = await client.listModels(); + const defaultModel = models.find((model) => model.isDefault) ?? models[0]; + if (defaultModel) { + normalized.model = defaultModel.id; + } + } catch { + // Provider may not support model listing — leave model undefined + } + } + } + + if (!normalized.modeId) { + try { + normalized.modeId = + getAgentProviderDefinition(normalized.provider).defaultModeId ?? undefined; + } catch { + // Unknown provider + } } return normalized; diff --git a/packages/server/src/server/agent/agent-mcp.e2e.test.ts b/packages/server/src/server/agent/agent-mcp.e2e.test.ts index c565f3b6c..ad95ce1b6 100644 --- a/packages/server/src/server/agent/agent-mcp.e2e.test.ts +++ b/packages/server/src/server/agent/agent-mcp.e2e.test.ts @@ -151,8 +151,8 @@ describe("agent MCP end-to-end (offline)", () => { args: { cwd: agentCwd, title: "MCP e2e smoke", - agentType: "claude", - initialMode: "bypassPermissions", + provider: "claude", + mode: "bypassPermissions", initialPrompt, background: false, }, @@ -242,8 +242,8 @@ describe("agent MCP end-to-end (offline)", () => { args: { cwd: agentCwd, title: "Injected MCP", - agentType: "claude", - initialMode: "bypassPermissions", + provider: "claude", + mode: "bypassPermissions", initialPrompt: "reply with done and stop", background: true, }, @@ -265,8 +265,8 @@ describe("agent MCP end-to-end (offline)", () => { args: { cwd: disabledAgentCwd, title: "No injected MCP", - agentType: "claude", - initialMode: "bypassPermissions", + provider: "claude", + mode: "bypassPermissions", initialPrompt: "reply with done and stop", background: true, }, @@ -362,8 +362,8 @@ describe("agent MCP end-to-end (offline)", () => { args: { cwd: repoRoot, title: "MCP worktree setup terminals", - agentType: "claude", - initialMode: "bypassPermissions", + provider: "claude", + mode: "bypassPermissions", initialPrompt: "say done and stop", worktreeName: "mcp-worktree-setup-test", baseBranch: "main", diff --git a/packages/server/src/server/agent/mcp-parity.e2e.test.ts b/packages/server/src/server/agent/mcp-parity.e2e.test.ts index 3c4758424..d949f72d0 100644 --- a/packages/server/src/server/agent/mcp-parity.e2e.test.ts +++ b/packages/server/src/server/agent/mcp-parity.e2e.test.ts @@ -121,9 +121,9 @@ describe("MCP parity end-to-end", () => { const payload = await callToolStructured(topLevelClient, "create_agent", { cwd, title: "Parity agent", - agentType: "claude", + provider: "claude", initialPrompt: "say done and stop", - initialMode: "bypassPermissions", + mode: "bypassPermissions", background: true, ...args, }); @@ -133,7 +133,7 @@ describe("MCP parity end-to-end", () => { async function createChildAgent(args?: Partial<StructuredContent>): Promise<string> { const payload = await callToolStructured(agentScopedClient, "create_agent", { title: "Parity child", - agentType: "claude", + provider: "claude", initialPrompt: "say done and stop", background: true, ...args, @@ -207,9 +207,9 @@ describe("MCP parity end-to-end", () => { const parentPayload = await callToolStructured(topLevelClient, "create_agent", { cwd: parentAgentCwd, title: "MCP parity parent", - agentType: "claude", + provider: "claude", initialPrompt: "say done and stop", - initialMode: "bypassPermissions", + mode: "bypassPermissions", background: true, }); parentAgentId = parentPayload.agentId as string; diff --git a/packages/server/src/server/agent/mcp-server.test.ts b/packages/server/src/server/agent/mcp-server.test.ts index e4c471ebf..2d3a649dc 100644 --- a/packages/server/src/server/agent/mcp-server.test.ts +++ b/packages/server/src/server/agent/mcp-server.test.ts @@ -65,7 +65,7 @@ describe("create_agent MCP tool", () => { const missingTitle = await tool.inputSchema.safeParseAsync({ cwd: existingCwd, - initialMode: "default", + mode: "default", initialPrompt: "test", }); expect(missingTitle.success).toBe(false); @@ -73,7 +73,7 @@ describe("create_agent MCP tool", () => { const tooLong = await tool.inputSchema.safeParseAsync({ cwd: existingCwd, - initialMode: "default", + mode: "default", title: "x".repeat(61), initialPrompt: "test", }); @@ -82,7 +82,7 @@ describe("create_agent MCP tool", () => { const ok = await tool.inputSchema.safeParseAsync({ cwd: existingCwd, - initialMode: "default", + mode: "default", title: "Short title", initialPrompt: "test", }); @@ -95,7 +95,7 @@ describe("create_agent MCP tool", () => { const tool = (server as any)._registeredTools["create_agent"]; const parsed = await tool.inputSchema.safeParseAsync({ cwd: existingCwd, - initialMode: "default", + mode: "default", title: "Short title", }); expect(parsed.success).toBe(false); @@ -194,7 +194,7 @@ describe("create_agent MCP tool", () => { await tool.callback({ cwd: existingCwd, title: "Config test", - initialMode: "default", + mode: "default", initialPrompt: "Do work", model: "claude-sonnet-4-20250514", thinking: "think-hard", @@ -248,7 +248,7 @@ describe("create_agent MCP tool", () => { await tool.callback({ cwd: "subdir", title: "Child", - agentType: "codex", + provider: "codex", initialPrompt: "Do work", }); @@ -287,7 +287,7 @@ describe("create_agent MCP tool", () => { await tool.callback({ cwd: existingCwd, title: "Injected config test", - initialMode: "default", + mode: "default", initialPrompt: "Do work", }); diff --git a/packages/server/src/server/agent/mcp-server.ts b/packages/server/src/server/agent/mcp-server.ts index 019f674c0..422674651 100644 --- a/packages/server/src/server/agent/mcp-server.ts +++ b/packages/server/src/server/agent/mcp-server.ts @@ -284,7 +284,7 @@ export async function createAgentMcpServer(options: AgentMcpServerOptions): Prom .min(1, "Title is required") .max(60, "Title must be 60 characters or fewer") .describe("Short descriptive title (<= 60 chars) summarizing the agent's focus."), - agentType: AgentProviderEnum.optional().describe( + provider: AgentProviderEnum.optional().describe( "Optional agent implementation to spawn. Defaults to 'claude'.", ), model: z.string().optional().describe("Model to use (e.g. claude-sonnet-4-20250514)"), @@ -321,7 +321,7 @@ export async function createAgentMcpServer(options: AgentMcpServerOptions): Prom .min(1, "Title is required") .max(60, "Title must be 60 characters or fewer") .describe("Short descriptive title (<= 60 chars) summarizing the agent's focus."), - agentType: AgentProviderEnum.optional().describe( + provider: AgentProviderEnum.optional().describe( "Optional agent implementation to spawn. Defaults to 'claude'.", ), model: z.string().optional().describe("Model to use (e.g. claude-sonnet-4-20250514)"), @@ -332,7 +332,10 @@ export async function createAgentMcpServer(options: AgentMcpServerOptions): Prom .trim() .min(1, "initialPrompt is required") .describe("Required first task to run immediately after creation."), - initialMode: z.string().describe("Required session mode to configure before the first run."), + mode: z + .string() + .optional() + .describe("Optional session mode to configure before the first run."), worktreeName: z .string() .optional() @@ -359,10 +362,7 @@ export async function createAgentMcpServer(options: AgentMcpServerOptions): Prom const createAgentInputSchema = callerAgentId ? agentToAgentInputSchema : topLevelInputSchema; const agentToAgentCreateAgentArgsSchema = z.object(agentToAgentInputSchema); - const topLevelCreateAgentArgsSchema = z.object({ - ...topLevelInputSchema, - initialMode: topLevelInputSchema.initialMode.optional(), - }); + const topLevelCreateAgentArgsSchema = z.object(topLevelInputSchema); if (options.voiceOnly || options.enableVoiceTools || callerContext?.enableVoiceTools) { server.registerTool( @@ -447,7 +447,7 @@ export async function createAgentMcpServer(options: AgentMcpServerOptions): Prom if (callerAgentId) { const callerArgs = agentToAgentCreateAgentArgsSchema.parse(args); - provider = callerArgs.agentType ?? "claude"; + provider = callerArgs.provider ?? "claude"; initialPrompt = callerArgs.initialPrompt; background = callerArgs.background ?? false; normalizedTitle = callerArgs.title.trim(); @@ -472,7 +472,7 @@ export async function createAgentMcpServer(options: AgentMcpServerOptions): Prom } } else { const topLevelArgs = topLevelCreateAgentArgsSchema.parse(args); - provider = topLevelArgs.agentType ?? "claude"; + provider = topLevelArgs.provider ?? "claude"; initialPrompt = topLevelArgs.initialPrompt; background = topLevelArgs.background ?? false; normalizedTitle = topLevelArgs.title.trim(); @@ -480,7 +480,7 @@ export async function createAgentMcpServer(options: AgentMcpServerOptions): Prom thinking = topLevelArgs.thinking; labels = topLevelArgs.labels; notifyOnFinish = topLevelArgs.notifyOnFinish ?? false; - const { cwd, initialMode, worktreeName, baseBranch } = topLevelArgs; + const { cwd, mode, worktreeName, baseBranch } = topLevelArgs; resolvedCwd = expandUserPath(cwd); @@ -499,7 +499,7 @@ export async function createAgentMcpServer(options: AgentMcpServerOptions): Prom worktreeConfig = worktree; } - resolvedMode = initialMode; + resolvedMode = mode; } const childAgentDefaultLabels = callerContext?.childAgentDefaultLabels; diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index cddd79008..b862666ad 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -1006,10 +1006,10 @@ async function getAheadOfOrigin(cwd: string, currentBranch: string): Promise<num return Number.isNaN(count) ? null : count; } catch { try { - const { stdout } = await execAsync( - `git rev-list --count ${currentBranch}`, - { cwd, env: READ_ONLY_GIT_ENV }, - ); + const { stdout } = await execAsync(`git rev-list --count ${currentBranch}`, { + cwd, + env: READ_ONLY_GIT_ENV, + }); const count = Number.parseInt(stdout.trim(), 10); return Number.isNaN(count) ? null : count; } catch {