diff --git a/apps/server/src/providers/claude-provider.ts b/apps/server/src/providers/claude-provider.ts index 92b0fdf7..2eceb422 100644 --- a/apps/server/src/providers/claude-provider.ts +++ b/apps/server/src/providers/claude-provider.ts @@ -10,7 +10,7 @@ import { BaseProvider } from './base-provider.js'; import { classifyError, getUserFriendlyErrorMessage, createLogger } from '@automaker/utils'; const logger = createLogger('ClaudeProvider'); -import { getThinkingTokenBudget } from '@automaker/types'; +import { getThinkingTokenBudget, validateBareModelId } from '@automaker/types'; import type { ExecuteOptions, ProviderMessage, @@ -53,6 +53,10 @@ export class ClaudeProvider extends BaseProvider { * Execute a query using Claude Agent SDK */ async *executeQuery(options: ExecuteOptions): AsyncGenerator { + // Validate that model doesn't have a provider prefix + // AgentService should strip prefixes before passing to providers + validateBareModelId(options.model, 'ClaudeProvider'); + const { prompt, model, diff --git a/apps/server/src/providers/codex-provider.ts b/apps/server/src/providers/codex-provider.ts index 858ff206..54e13989 100644 --- a/apps/server/src/providers/codex-provider.ts +++ b/apps/server/src/providers/codex-provider.ts @@ -31,6 +31,7 @@ import type { import { CODEX_MODEL_MAP, supportsReasoningEffort, + validateBareModelId, type CodexApprovalPolicy, type CodexSandboxMode, type CodexAuthStatus, @@ -663,6 +664,10 @@ export class CodexProvider extends BaseProvider { } async *executeQuery(options: ExecuteOptions): AsyncGenerator { + // Validate that model doesn't have a provider prefix + // AgentService should strip prefixes before passing to providers + validateBareModelId(options.model, 'CodexProvider'); + try { const mcpServers = options.mcpServers ?? {}; const hasMcpServers = Object.keys(mcpServers).length > 0; @@ -760,6 +765,7 @@ export class CodexProvider extends BaseProvider { } } + // Model is already bare (no prefix) - validated by executeQuery const args = [ CODEX_EXEC_SUBCOMMAND, CODEX_YOLO_FLAG, diff --git a/apps/server/src/providers/cursor-provider.ts b/apps/server/src/providers/cursor-provider.ts index aedae441..6cefc279 100644 --- a/apps/server/src/providers/cursor-provider.ts +++ b/apps/server/src/providers/cursor-provider.ts @@ -28,7 +28,7 @@ import type { ModelDefinition, ContentBlock, } from './types.js'; -import { stripProviderPrefix } from '@automaker/types'; +import { validateBareModelId } from '@automaker/types'; import { validateApiKey } from '../lib/auth-utils.js'; import { getEffectivePermissions } from '../services/cursor-config-service.js'; import { @@ -317,8 +317,8 @@ export class CursorProvider extends CliProvider { } buildCliArgs(options: ExecuteOptions): string[] { - // Extract model (strip 'cursor-' prefix if present) - const model = stripProviderPrefix(options.model || 'auto'); + // Model is already bare (no prefix) - validated by executeQuery + const model = options.model || 'auto'; // Build CLI arguments for cursor-agent // NOTE: Prompt is NOT included here - it's passed via stdin to avoid @@ -649,6 +649,10 @@ export class CursorProvider extends CliProvider { async *executeQuery(options: ExecuteOptions): AsyncGenerator { this.ensureCliDetected(); + // Validate that model doesn't have a provider prefix + // AgentService should strip prefixes before passing to providers + validateBareModelId(options.model, 'CursorProvider'); + if (!this.cliPath) { throw this.createError( CursorErrorCode.NOT_INSTALLED, diff --git a/apps/server/src/services/agent-service.ts b/apps/server/src/services/agent-service.ts index 5e29d0db..abf4c0ab 100644 --- a/apps/server/src/services/agent-service.ts +++ b/apps/server/src/services/agent-service.ts @@ -7,6 +7,7 @@ import path from 'path'; import * as secureFs from '../lib/secure-fs.js'; import type { EventEmitter } from '../lib/events.js'; import type { ExecuteOptions, ThinkingLevel, ReasoningEffort } from '@automaker/types'; +import { stripProviderPrefix } from '@automaker/types'; import { readImageAsBase64, buildPromptWithImages, @@ -290,13 +291,17 @@ export class AgentService { const maxTurns = sdkOptions.maxTurns; const allowedTools = sdkOptions.allowedTools as string[] | undefined; - // Get provider for this model + // Get provider for this model (with prefix) const provider = ProviderFactory.getProviderForModel(effectiveModel); + // Strip provider prefix - providers should receive bare model IDs + const bareModel = stripProviderPrefix(effectiveModel); + // Build options for provider const options: ExecuteOptions = { prompt: '', // Will be set below based on images - model: effectiveModel, + model: bareModel, // Bare model ID (e.g., "gpt-5.1-codex-max", "composer-1") + originalModel: effectiveModel, // Original with prefix for logging (e.g., "codex-gpt-5.1-codex-max") cwd: effectiveWorkDir, systemPrompt: sdkOptions.systemPrompt, maxTurns: maxTurns, diff --git a/apps/server/src/services/ideation-service.ts b/apps/server/src/services/ideation-service.ts index 7973db05..81fc3de6 100644 --- a/apps/server/src/services/ideation-service.ts +++ b/apps/server/src/services/ideation-service.ts @@ -40,6 +40,7 @@ import type { SettingsService } from './settings-service.js'; import type { FeatureLoader } from './feature-loader.js'; import { createChatOptions, validateWorkingDirectory } from '../lib/sdk-options.js'; import { resolveModelString } from '@automaker/model-resolver'; +import { stripProviderPrefix } from '@automaker/types'; const logger = createLogger('IdeationService'); @@ -201,7 +202,7 @@ export class IdeationService { existingWorkContext ); - // Resolve model alias to canonical identifier + // Resolve model alias to canonical identifier (with prefix) const modelId = resolveModelString(options?.model ?? 'sonnet'); // Create SDK options @@ -214,9 +215,13 @@ export class IdeationService { const provider = ProviderFactory.getProviderForModel(modelId); + // Strip provider prefix - providers need bare model IDs + const bareModel = stripProviderPrefix(modelId); + const executeOptions: ExecuteOptions = { prompt: message, - model: modelId, + model: bareModel, + originalModel: modelId, cwd: projectPath, systemPrompt: sdkOptions.systemPrompt, maxTurns: 1, // Single turn for ideation @@ -648,7 +653,7 @@ export class IdeationService { existingWorkContext ); - // Resolve model alias to canonical identifier + // Resolve model alias to canonical identifier (with prefix) const modelId = resolveModelString('sonnet'); // Create SDK options @@ -661,9 +666,13 @@ export class IdeationService { const provider = ProviderFactory.getProviderForModel(modelId); + // Strip provider prefix - providers need bare model IDs + const bareModel = stripProviderPrefix(modelId); + const executeOptions: ExecuteOptions = { prompt: prompt.prompt, - model: modelId, + model: bareModel, + originalModel: modelId, cwd: projectPath, systemPrompt: sdkOptions.systemPrompt, maxTurns: 1, diff --git a/apps/server/tests/unit/providers/claude-provider.test.ts b/apps/server/tests/unit/providers/claude-provider.test.ts index a02d3b5a..f107c4f4 100644 --- a/apps/server/tests/unit/providers/claude-provider.test.ts +++ b/apps/server/tests/unit/providers/claude-provider.test.ts @@ -37,6 +37,7 @@ describe('claude-provider.ts', () => { const generator = provider.executeQuery({ prompt: 'Hello', + model: 'claude-opus-4-5-20251101', cwd: '/test', }); @@ -88,6 +89,7 @@ describe('claude-provider.ts', () => { const generator = provider.executeQuery({ prompt: 'Test', + model: 'claude-opus-4-5-20251101', cwd: '/test', }); @@ -112,6 +114,7 @@ describe('claude-provider.ts', () => { const generator = provider.executeQuery({ prompt: 'Test', + model: 'claude-opus-4-5-20251101', cwd: '/test', abortController, }); @@ -140,6 +143,7 @@ describe('claude-provider.ts', () => { const generator = provider.executeQuery({ prompt: 'Current message', + model: 'claude-opus-4-5-20251101', cwd: '/test', conversationHistory, sdkSessionId: 'test-session-id', @@ -170,6 +174,7 @@ describe('claude-provider.ts', () => { const generator = provider.executeQuery({ prompt: arrayPrompt as any, + model: 'claude-opus-4-5-20251101', cwd: '/test', }); @@ -189,6 +194,7 @@ describe('claude-provider.ts', () => { const generator = provider.executeQuery({ prompt: 'Test', + model: 'claude-opus-4-5-20251101', cwd: '/test', }); @@ -214,6 +220,7 @@ describe('claude-provider.ts', () => { const generator = provider.executeQuery({ prompt: 'Test', + model: 'claude-opus-4-5-20251101', cwd: '/test', }); diff --git a/libs/types/src/index.ts b/libs/types/src/index.ts index edc7dd0b..ec1a57d0 100644 --- a/libs/types/src/index.ts +++ b/libs/types/src/index.ts @@ -185,6 +185,7 @@ export { addProviderPrefix, getBareModelId, normalizeModelString, + validateBareModelId, } from './provider-utils.js'; // Pipeline types diff --git a/libs/types/src/provider-utils.ts b/libs/types/src/provider-utils.ts index 143420c6..1842d12c 100644 --- a/libs/types/src/provider-utils.ts +++ b/libs/types/src/provider-utils.ts @@ -196,3 +196,37 @@ export function normalizeModelString(model: string | undefined | null): string { return model; } + +/** + * Validate that a model ID does not contain a provider prefix + * + * Providers should receive bare model IDs (e.g., "gpt-5.1-codex-max", "composer-1") + * without provider prefixes (e.g., NOT "codex-gpt-5.1-codex-max", NOT "cursor-composer-1"). + * + * This validation ensures the ProviderFactory properly stripped prefixes before + * passing models to providers. + * + * @param model - Model ID to validate + * @param providerName - Name of the provider for error messages + * @throws Error if model contains a provider prefix + * + * @example + * validateBareModelId("gpt-5.1-codex-max", "CodexProvider"); // ✅ OK + * validateBareModelId("codex-gpt-5.1-codex-max", "CodexProvider"); // ❌ Throws error + */ +export function validateBareModelId(model: string, providerName: string): void { + if (!model || typeof model !== 'string') { + throw new Error(`[${providerName}] Invalid model ID: expected string, got ${typeof model}`); + } + + for (const [provider, prefix] of Object.entries(PROVIDER_PREFIXES)) { + if (model.startsWith(prefix)) { + throw new Error( + `[${providerName}] Model ID should not contain provider prefix '${prefix}'. ` + + `Got: '${model}'. ` + + `This is likely a bug in ProviderFactory - it should strip the '${provider}' prefix ` + + `before passing the model to the provider.` + ); + } + } +} diff --git a/libs/types/src/provider.ts b/libs/types/src/provider.ts index 3c41259b..d9848a1b 100644 --- a/libs/types/src/provider.ts +++ b/libs/types/src/provider.ts @@ -81,7 +81,10 @@ export interface McpHttpServerConfig { */ export interface ExecuteOptions { prompt: string | Array<{ type: string; text?: string; source?: object }>; + /** Bare model ID without provider prefix (e.g., "gpt-5.1-codex-max", "composer-1") */ model: string; + /** Original model ID with provider prefix for logging (e.g., "codex-gpt-5.1-codex-max") */ + originalModel?: string; cwd: string; systemPrompt?: string | SystemPromptPreset; maxTurns?: number;