diff --git a/docs/users/features/sub-agents.md b/docs/users/features/sub-agents.md index d4473572efb..59b60bf75ba 100644 --- a/docs/users/features/sub-agents.md +++ b/docs/users/features/sub-agents.md @@ -134,7 +134,7 @@ Subagents are configured using Markdown files with YAML frontmatter. This format --- name: agent-name description: Brief description of when and how to use this agent -model: inherit # Optional: inherit or model-id +model: inherit # Optional: inherit, fast, modelId, or authType:modelId approvalMode: auto-edit # Optional: default, plan, auto-edit, yolo tools: # Optional: allowlist of tools - tool1 @@ -151,10 +151,48 @@ Multiple paragraphs are supported. Use the optional `model` frontmatter field to control which model a subagent uses: -- `inherit`: Use the same model as the main conversation -- Omit the field: Same as `inherit` -- `glm-5`: Use that model ID with the main conversation's auth type -- `openai:gpt-4o`: Use a different provider (resolves credentials from env vars) +- `inherit`: Use the same model as the main conversation. +- Omit the field: Same as `inherit`. +- `fast`: Use the configured `fastModel`. If no valid fast model is configured, + the subagent falls back to `inherit`. +- `glm-5`: Use that model ID. Qwen Code first checks the main conversation's + auth type; if the model is not available there, it can resolve the model from + another configured provider. +- `openai:gpt-4o`: Use an explicit provider and model ID. This is useful when a + subagent should run on a model registered under a different auth type from the + main conversation. + +For example: + +``` +--- +name: fast-reviewer +description: Reviews small diffs with the configured fast model +model: fast +tools: + - read_file + - grep_search +--- +``` + +``` +--- +name: openai-researcher +description: Uses an OpenAI-compatible provider for research tasks +model: openai:gpt-4o +tools: + - read_file + - grep_search + - glob +--- +``` + +The `fast` selector uses the same `fastModel` setting configured in +`settings.json` or with `/model --fast`. That setting may itself refer to a +model under another configured auth type, such as `openai:deepseek-v4-flash`. +When the selector resolves to another auth type, Qwen Code creates a dedicated +runtime provider for that subagent request and sends the provider only the bare +model ID. #### Permission Mode @@ -620,6 +658,10 @@ Always follow these standards: - **Tool Restrictions**: Use `tools` to limit which tools a subagent can access, or `disallowedTools` to block specific tools while inheriting everything else - **Permission Mode**: Subagents inherit their parent's permission mode by default. Plan-mode sessions cannot escalate to auto-edit through delegated agents. Privileged modes (auto-edit, yolo) are blocked in untrusted folders. +- **Provider Selection**: A subagent with `model: authType:modelId`, or + `model: fast` where `fastModel` resolves to another auth type, sends that + subagent's model requests to the selected provider. Make sure that provider is + appropriate for the subagent's task and data. - **Sandboxing**: All tool execution follows the same security model as direct tool use - **Audit Trail**: All Subagents actions are logged and visible in real-time - **Access Control**: Project and user-level separation provides appropriate boundaries diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index b970093f150..62f1cb79e05 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -1899,10 +1899,8 @@ export const AppContainer = (props: AppContainerProps) => { const fullHistory = geminiClient.getChat().getHistory(true); const conversationHistory = fullHistory.length > 40 ? fullHistory.slice(-40) : fullHistory; - const fastModel = config.getFastModel(); generatePromptSuggestion(config, conversationHistory, ac.signal, { enableCacheSharing: settings.merged.ui?.enableCacheSharing === true, - model: fastModel, }) .then((result) => { if (ac.signal.aborted) return; @@ -1910,9 +1908,7 @@ export const AppContainer = (props: AppContainerProps) => { setPromptSuggestion(result.suggestion); // Start speculation if enabled (runs in background) if (settings.merged.ui?.enableSpeculation) { - startSpeculation(config, result.suggestion, ac.signal, { - model: fastModel, - }) + startSpeculation(config, result.suggestion, ac.signal) .then((state) => { speculationRef.current = state; }) diff --git a/packages/cli/src/ui/commands/recapCommand.ts b/packages/cli/src/ui/commands/recapCommand.ts index a56b52533b8..5231a376d18 100644 --- a/packages/cli/src/ui/commands/recapCommand.ts +++ b/packages/cli/src/ui/commands/recapCommand.ts @@ -63,12 +63,12 @@ export const recapCommand: SlashCommand = { if (context.executionMode === 'interactive') { const item: HistoryItemAwayRecap = { type: 'away_recap', - text: recap.text, + text: recap, }; context.ui.addItem(item, Date.now()); return; } - return { type: 'message', messageType: 'info', content: recap.text }; + return { type: 'message', messageType: 'info', content: recap }; }, }; diff --git a/packages/cli/src/ui/hooks/useAwaySummary.test.ts b/packages/cli/src/ui/hooks/useAwaySummary.test.ts index d5c28076deb..3d81d974dcb 100644 --- a/packages/cli/src/ui/hooks/useAwaySummary.test.ts +++ b/packages/cli/src/ui/hooks/useAwaySummary.test.ts @@ -54,10 +54,7 @@ describe('useAwaySummary', () => { const recordSlashCommand = vi.fn(); const config = makeConfig(recordSlashCommand); const addItem = vi.fn(); - generateSessionRecapMock.mockResolvedValue({ - text: 'recap text', - modelUsed: 'fast', - }); + generateSessionRecapMock.mockResolvedValue('recap text'); // Mount blurred to set the away-start timestamp. const { rerender } = renderHook( @@ -104,10 +101,7 @@ describe('useAwaySummary', () => { const recordSlashCommand = vi.fn(); const config = makeConfig(recordSlashCommand); const addItem = vi.fn(); - generateSessionRecapMock.mockResolvedValue({ - text: 'should not appear', - modelUsed: 'fast', - }); + generateSessionRecapMock.mockResolvedValue('should not appear'); const historyWithRecentRecap: HistoryItem[] = [ ...THREE_USER_HISTORY, diff --git a/packages/cli/src/ui/hooks/useAwaySummary.ts b/packages/cli/src/ui/hooks/useAwaySummary.ts index 1c251ad41e0..53311ddb284 100644 --- a/packages/cli/src/ui/hooks/useAwaySummary.ts +++ b/packages/cli/src/ui/hooks/useAwaySummary.ts @@ -150,7 +150,7 @@ export function useAwaySummary(options: UseAwaySummaryOptions): void { if (!isIdleRef.current) return; const item: HistoryItemAwayRecap = { type: 'away_recap', - text: recap.text, + text: recap, }; addItem(item, Date.now()); diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 876228b26d6..dd00d7e4b79 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -867,7 +867,7 @@ describe('Server Config (config.ts)', () => { }); describe('model switching with different credentials (OpenAI)', () => { - it('keeps getFastModel current-auth-only for direct runtime callers', () => { + it('returns a bare fast model selector when the model is configured under another auth type', () => { const config = new Config({ ...baseParams, authType: AuthType.USE_ANTHROPIC, @@ -893,11 +893,10 @@ describe('Server Config (config.ts)', () => { }, }); - expect(config.getFastModel()).toBeUndefined(); - expect(config.getFastModelForSideQuery()).toBe('deepseek-v4-flash'); + expect(config.getFastModel()).toBe('deepseek-v4-flash'); }); - it('returns an authType-qualified fast model selector for side queries', () => { + it('returns an authType-qualified fast model selector', () => { const config = new Config({ ...baseParams, authType: AuthType.USE_ANTHROPIC, @@ -923,11 +922,10 @@ describe('Server Config (config.ts)', () => { }, }); - expect(config.getFastModel()).toBeUndefined(); - expect(config.getFastModelForSideQuery()).toBe('openai:shared-model'); + expect(config.getFastModel()).toBe('openai:shared-model'); }); - it('returns a bare fast model for getFastModel when authType-qualified selector matches the current auth type', () => { + it('keeps authType-qualified selectors when the auth type matches the current auth type', () => { const config = new Config({ ...baseParams, authType: AuthType.USE_OPENAI, @@ -945,10 +943,7 @@ describe('Server Config (config.ts)', () => { }, }); - expect(config.getFastModel()).toBe('deepseek-v4-flash'); - expect(config.getFastModelForSideQuery()).toBe( - 'openai:deepseek-v4-flash', - ); + expect(config.getFastModel()).toBe('openai:deepseek-v4-flash'); }); it('accepts runtime fast models for authType-qualified selectors', () => { @@ -979,10 +974,7 @@ describe('Server Config (config.ts)', () => { }); config.getModelsConfig().detectAndCaptureRuntimeModel(); - expect(config.getFastModel()).toBe('runtime-fast-model'); - expect(config.getFastModelForSideQuery()).toBe( - 'openai:runtime-fast-model', - ); + expect(config.getFastModel()).toBe('openai:runtime-fast-model'); }); it('returns undefined when the fast model is not configured for any auth type', () => { @@ -1004,7 +996,6 @@ describe('Server Config (config.ts)', () => { }); expect(config.getFastModel()).toBeUndefined(); - expect(config.getFastModelForSideQuery()).toBeUndefined(); }); it('returns undefined when the fast model selector is malformed', () => { @@ -1026,7 +1017,6 @@ describe('Server Config (config.ts)', () => { }); expect(config.getFastModel()).toBeUndefined(); - expect(config.getFastModelForSideQuery()).toBeUndefined(); }); it('returns undefined when fastModel points back to the fast selector', () => { @@ -1048,7 +1038,6 @@ describe('Server Config (config.ts)', () => { }); expect(config.getFastModel()).toBeUndefined(); - expect(config.getFastModelForSideQuery()).toBeUndefined(); }); it('should refresh auth when switching to model with different envKey', async () => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 94e769a555a..2229e6f17cc 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1861,52 +1861,35 @@ export class Config { } /** - * Returns the fast model if one is configured and valid for the current auth - * type, otherwise returns undefined. Direct runtime paths use this as a - * cheaper alternative to the main session model, so it intentionally stays - * current-auth-only. + * Returns the configured fast model selector when it resolves to an available + * model. Bare selectors stay bare and authType-qualified selectors keep their + * authType prefix so selector-aware runtime paths can route cross-auth calls. */ getFastModel(): string | undefined { - const authType = - this.contentGeneratorConfig?.authType ?? - this.modelsConfig.getCurrentAuthType(); - if (!authType) return undefined; const selector = this.resolveFastModelSelector(); if (!selector) return undefined; - if (selector.authType && selector.authType !== authType) return undefined; - const available = this.getAllConfiguredModels([authType]); - return available.some((m) => m.id === selector.modelId) - ? selector.modelId - : undefined; - } - - /** - * Returns the fast model for side-query paths. Unlike {@link getFastModel}, - * this can return an authType-qualified selector because BaseLlmClient can - * route a single request through a provider different from the main session. - */ - getFastModelForSideQuery(): string | undefined { - const selector = this.resolveFastModelSelector(); - if (!selector) return undefined; - - if (selector.authType) { - const available = this.getAllConfiguredModels([selector.authType]); - return available.some((m) => m.id === selector.modelId) - ? `${selector.authType}:${selector.modelId}` - : undefined; + const available = selector.authType + ? this.getAllConfiguredModels([selector.authType]) + : this.getAllConfiguredModels(); + if (!available.some((m) => m.id === selector.modelId)) { + return undefined; } - const available = this.getAllConfiguredModels(); - return available.some((m) => m.id === selector.modelId) - ? selector.modelId - : undefined; + const rawSelector = resolveModelId(this.fastModel); + return rawSelector?.authType + ? `${rawSelector.authType}:${selector.modelId}` + : selector.modelId; } private resolveFastModelSelector() { if (!this.fastModel) return undefined; try { - return resolveModelId(this.fastModel); + return resolveModelId(this.fastModel, { + currentAuthType: this.getContentGeneratorConfig()?.authType, + getAvailableModels: (authTypes) => + this.getAllConfiguredModels(authTypes), + }); } catch { return undefined; } diff --git a/packages/core/src/core/baseLlmClient.test.ts b/packages/core/src/core/baseLlmClient.test.ts index 31a7321e113..69048de25d5 100644 --- a/packages/core/src/core/baseLlmClient.test.ts +++ b/packages/core/src/core/baseLlmClient.test.ts @@ -559,7 +559,16 @@ describe('BaseLlmClient', () => { getEmbeddingModel: vi.fn().mockReturnValue('test-embedding-model'), getModel: vi.fn().mockReturnValue('main-model'), getFastModel: vi.fn().mockReturnValue(undefined), - getFastModelForSideQuery: vi.fn().mockReturnValue(undefined), + getAllConfiguredModels: vi.fn((authTypes?: AuthType[]) => + authTypes?.includes(AuthType.QWEN_OAUTH) + ? [] + : [ + { + id: fastModel, + authType: AuthType.USE_ANTHROPIC, + }, + ], + ), getModelsConfig: vi.fn().mockReturnValue({ getResolvedModel }), } as unknown as Mocked; }); @@ -575,6 +584,29 @@ describe('BaseLlmClient', () => { expect(mockCreateContentGenerator).not.toHaveBeenCalled(); }); + it('returns the active runtime generator when model matches the runtime view', async () => { + const runtimeContentGenerator = { + generateContent: vi.fn(), + embedContent: vi.fn(), + } as unknown as Mocked; + crossProviderConfig.getContentGenerator = vi + .fn() + .mockReturnValue(runtimeContentGenerator); + vi.mocked(crossProviderConfig.getContentGeneratorConfig).mockReturnValue({ + authType: AuthType.USE_OPENAI, + model: 'runtime-model', + }); + vi.mocked(crossProviderConfig.getModel).mockReturnValue('runtime-model'); + const c = new BaseLlmClient(mockContentGenerator, crossProviderConfig); + + const resolved = await c.resolveForModel('runtime-model'); + + expect(resolved.contentGenerator).toBe(runtimeContentGenerator); + expect(resolved.retryAuthType).toBe(AuthType.USE_OPENAI); + expect(getResolvedModel).not.toHaveBeenCalled(); + expect(mockCreateContentGenerator).not.toHaveBeenCalled(); + }); + it('builds a per-model generator when model differs and is registered under another authType', async () => { // Main authType is QWEN_OAUTH; fast model only resolves under USE_ANTHROPIC. getResolvedModel.mockImplementation((authType: string, model: string) => { @@ -646,6 +678,38 @@ describe('BaseLlmClient', () => { expect(mockCreateContentGenerator).not.toHaveBeenCalled(); }); + it('does not cache the unregistered-model fallback across runtime-view changes', async () => { + // Unregistered selector: createContentGeneratorForModel falls back to + // getCurrentContentGenerator(). The runtime view changes between calls + // — caching would pin the first call's generator under the selector + // key and return it on the second call after the view has unwound. + getResolvedModel.mockReturnValue(undefined); + + const firstRuntimeGenerator = { + generateContent: vi.fn(), + embedContent: vi.fn(), + } as unknown as Mocked; + const secondRuntimeGenerator = { + generateContent: vi.fn(), + embedContent: vi.fn(), + } as unknown as Mocked; + const getContentGenerator = vi + .fn() + .mockReturnValueOnce(firstRuntimeGenerator) + .mockReturnValueOnce(secondRuntimeGenerator); + crossProviderConfig.getContentGenerator = getContentGenerator; + + const c = new BaseLlmClient(mockContentGenerator, crossProviderConfig); + + const first = await c.resolveForModel('unknown-model'); + const second = await c.resolveForModel('unknown-model'); + + expect(first.contentGenerator).toBe(firstRuntimeGenerator); + expect(second.contentGenerator).toBe(secondRuntimeGenerator); + expect(getContentGenerator).toHaveBeenCalledTimes(2); + expect(mockCreateContentGenerator).not.toHaveBeenCalled(); + }); + it('falls back to the main generator when createContentGenerator throws', async () => { getResolvedModel.mockReturnValue({ authType: AuthType.USE_ANTHROPIC, @@ -738,9 +802,7 @@ describe('BaseLlmClient', () => { }); it('generateJson resolves fast selectors through the configured fast model', async () => { - crossProviderConfig.getFastModelForSideQuery.mockReturnValue( - 'openai:shared-model', - ); + crossProviderConfig.getFastModel.mockReturnValue('openai:shared-model'); getResolvedModel.mockImplementation((authType: string, model: string) => { if (authType === AuthType.USE_OPENAI && model === 'shared-model') { return { diff --git a/packages/core/src/core/baseLlmClient.ts b/packages/core/src/core/baseLlmClient.ts index aa5276da144..b2e56596b99 100644 --- a/packages/core/src/core/baseLlmClient.ts +++ b/packages/core/src/core/baseLlmClient.ts @@ -19,7 +19,11 @@ import type { ContentGenerator } from './contentGenerator.js'; import { AuthType, createContentGenerator } from './contentGenerator.js'; import type { ResolvedModelConfig } from '../models/types.js'; import { buildAgentContentGeneratorConfig } from '../models/content-generator-config.js'; -import { resolveModelId, type ResolvedModelId } from '../utils/modelId.js'; +import { + buildModelIdContext, + resolveModelId, + type ResolvedModelId, +} from '../utils/modelId.js'; import { reportError } from '../utils/errorReporting.js'; import { getErrorMessage } from '../utils/errors.js'; import { retryWithBackoff, isUnattendedMode } from '../utils/retry.js'; @@ -172,6 +176,10 @@ export class BaseLlmClient { private readonly config: Config, ) {} + private getCurrentContentGenerator(): ContentGenerator { + return this.config.getContentGenerator?.() ?? this.contentGenerator; + } + async generateJson( options: GenerateJsonOptions, ): Promise> { @@ -416,7 +424,7 @@ export class BaseLlmClient { (!selector?.authType || selector.authType === mainAuthType) ) { return { - contentGenerator: this.contentGenerator, + contentGenerator: this.getCurrentContentGenerator(), retryAuthType: mainAuthType, model: requestModel, }; @@ -496,17 +504,21 @@ export class BaseLlmClient { const cached = this.perModelGeneratorCache.get(cacheKey); if (cached) return cached; - const generatorPromise = (async () => { - try { - const resolvedModel = this.resolveModelAcrossAuthTypes(model, selector); + const resolvedModel = this.resolveModelAcrossAuthTypes(model, selector); - if (!resolvedModel) { - debugLogger.warn( - `Model "${model}" not found in registry across all authTypes, falling back to main generator.`, - ); - return this.contentGenerator; - } + if (!resolvedModel) { + debugLogger.warn( + `Model "${model}" not found in registry across all authTypes, falling back to main generator.`, + ); + // Do not cache the fallback: getCurrentContentGenerator() reads the + // runtime view from AsyncLocalStorage, which can differ between calls + // (e.g. inside a subagent vs. on the main session). Caching here would + // pin the first-call view's generator under this selector key. + return this.getCurrentContentGenerator(); + } + const generatorPromise = (async () => { + try { const targetModel = resolvedModel.id ?? selector?.modelId ?? model; const targetConfig = buildAgentContentGeneratorConfig( this.config, @@ -527,7 +539,7 @@ export class BaseLlmClient { err instanceof Error ? err.message : String(err), ); this.perModelGeneratorCache.delete(cacheKey); - return this.contentGenerator; + return this.getCurrentContentGenerator(); } })(); @@ -536,12 +548,6 @@ export class BaseLlmClient { } private resolveModelSelector(model: string): ResolvedModelId | undefined { - return resolveModelId(model, { - currentModel: this.config.getModel(), - currentAuthType: this.config.getContentGeneratorConfig()?.authType, - fastModel: - this.config.getFastModelForSideQuery?.() ?? - this.config.getFastModel?.(), - }); + return resolveModelId(model, buildModelIdContext(this.config)); } } diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index 57b268ce7ca..e403008dd69 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -462,6 +462,7 @@ describe('Gemini Client (client.ts)', () => { getModelsConfig: vi.fn().mockReturnValue({ getResolvedModel: vi.fn().mockReturnValue(undefined), }), + getAllConfiguredModels: vi.fn().mockReturnValue([]), getDisableAllHooks: vi.fn().mockReturnValue(true), getStopHookBlockingCap: vi.fn().mockReturnValue(8), getArenaManager: vi.fn().mockReturnValue(null), @@ -4948,15 +4949,26 @@ Other open files: envKey: undefined, }; - // resolveModelAcrossAuthTypes calls getResolvedModel multiple times: - // 1. main authType (QWEN_OAUTH) → undefined (miss) - // 2. secondary authType (USE_OPENAI) → mockResolvedModel (hit) - // 3. buildAgentContentGeneratorConfig calls getResolvedModel again - // with the resolved authType → mockResolvedModel (hit) - const getResolvedModel = vi - .fn() - .mockReturnValueOnce(undefined) - .mockReturnValue(mockResolvedModel); + // The central model-id resolver can now identify the authType from the + // configured model list before BaseLlmClient asks ModelsConfig for the + // concrete provider settings. + vi.mocked(mockConfig.getAllConfiguredModels).mockImplementation( + (authTypes?: AuthType[]) => + !authTypes || authTypes.includes(AuthType.USE_OPENAI) + ? [ + { + id: 'fast-model', + label: 'Fast Model', + authType: AuthType.USE_OPENAI, + }, + ] + : [], + ); + const getResolvedModel = vi.fn((authType: AuthType, model: string) => + authType === AuthType.USE_OPENAI && model === 'fast-model' + ? mockResolvedModel + : undefined, + ); vi.mocked(mockConfig.getModelsConfig).mockReturnValue({ getResolvedModel, @@ -4980,15 +4992,10 @@ Other open files: 'fast-model', ); - // First call uses main authType (QWEN_OAUTH) — misses + // The model-id resolver found the configured OpenAI owner, so + // ModelsConfig is queried directly with that authType. expect(getResolvedModel).toHaveBeenNthCalledWith( 1, - AuthType.QWEN_OAUTH, - 'fast-model', - ); - // Second call falls through to secondary authType — hits - expect(getResolvedModel).toHaveBeenNthCalledWith( - 2, AuthType.USE_OPENAI, 'fast-model', ); diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts index aaf3e1b2071..a9aa2e1f852 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts @@ -348,7 +348,7 @@ describe('LoggingContentGenerator', () => { }; const generator = new LoggingContentGenerator( wrapped, - createConfig(), + createConfig({ authType: AuthType.USE_ANTHROPIC }), generatorConfig, ); @@ -401,6 +401,7 @@ describe('LoggingContentGenerator', () => { expect(responseEvent.response_id).toBe('resp-1'); expect(responseEvent.model).toBe('model-v2'); expect(responseEvent.prompt_id).toBe('prompt-1'); + expect(responseEvent.auth_type).toBe(AuthType.USE_OPENAI); expect(responseEvent.input_token_count).toBe(3); expect(responseEvent.response_text).toBe('ok'); diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts index b1718772a7a..c4f3b80b75e 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts @@ -75,6 +75,7 @@ export class LoggingContentGenerator implements ContentGenerator { private openaiLogger?: OpenAILogger; private schemaCompliance?: 'auto' | 'openapi_30'; private modalities?: InputModalities; + private readonly generatorAuthType: ContentGeneratorConfig['authType']; constructor( private readonly wrapped: ContentGenerator, @@ -82,6 +83,7 @@ export class LoggingContentGenerator implements ContentGenerator { generatorConfig: ContentGeneratorConfig, ) { this.modalities = generatorConfig.modalities; + this.generatorAuthType = generatorConfig.authType; // Extract fields needed for initialization from passed config // (config.getContentGeneratorConfig() may not be available yet during refreshAuth) @@ -130,7 +132,7 @@ export class LoggingContentGenerator implements ContentGenerator { model, durationMs, prompt_id, - this.config.getAuthType(), + this.generatorAuthType, usageMetadata, responseText, subagentNameContext.getStore(), @@ -160,7 +162,7 @@ export class LoggingContentGenerator implements ContentGenerator { model, durationMs, promptId: prompt_id, - authType: this.config.getAuthType(), + authType: this.generatorAuthType, errorMessage, errorType, statusCode: errorStatus, diff --git a/packages/core/src/followup/speculation.ts b/packages/core/src/followup/speculation.ts index f26d7c9cb12..a6f06d73ca9 100644 --- a/packages/core/src/followup/speculation.ts +++ b/packages/core/src/followup/speculation.ts @@ -25,6 +25,7 @@ import { getCacheSafeParams, createForkedChat, runForkedAgent, + runWithForkedChatModel, } from '../utils/forkedAgent.js'; import { getFilterReason, SUGGESTION_PROMPT } from './suggestionGenerator.js'; @@ -200,199 +201,204 @@ async function runSpeculativeLoop( cacheSafe: import('../utils/forkedAgent.js').CacheSafeParams, modelOverride?: string, ): Promise { - const chat = createForkedChat(config, cacheSafe); - const model = modelOverride || cacheSafe.model; - const approvalMode = config.getApprovalMode(); - const messages: Content[] = []; - - // Add the suggestion as the initial user message - const userMsg: Content = { - role: 'user', - parts: [{ text: state.suggestion }], - }; - messages.push(userMsg); - - for (let turn = 0; turn < MAX_SPECULATION_TURNS; turn++) { - if (state.abortController?.signal.aborted) break; - if (messages.length >= MAX_SPECULATION_MESSAGES) break; - - // Send user message for this turn - const lastUserMsg = messages[messages.length - 1]; - const stream = await chat.sendMessageStream( - model, - { message: lastUserMsg.parts ?? [] }, - 'speculation', - ); + const modelSelector = + modelOverride ?? config.getFastModel() ?? cacheSafe.model; + return runWithForkedChatModel(config, modelSelector, async (model) => { + const chat = createForkedChat(config, cacheSafe); + const approvalMode = config.getApprovalMode(); + const messages: Content[] = []; + + // Add the suggestion as the initial user message + const userMsg: Content = { + role: 'user', + parts: [{ text: state.suggestion }], + }; + messages.push(userMsg); - const modelParts: Part[] = []; - for await (const event of stream) { + for (let turn = 0; turn < MAX_SPECULATION_TURNS; turn++) { if (state.abortController?.signal.aborted) break; - if (event.type !== StreamEventType.CHUNK) continue; - const response = event.value; - const parts = response.candidates?.[0]?.content?.parts ?? []; - for (const part of parts) { - // Skip thought/reasoning parts — only capture visible text + function calls - if (part.text && !(part as Record)['thought']) { - modelParts.push({ text: part.text }); - } - if (part.functionCall && part.functionCall.name) { - modelParts.push({ - functionCall: { - name: part.functionCall.name, - args: part.functionCall.args, - }, - }); + if (messages.length >= MAX_SPECULATION_MESSAGES) break; + + // Send user message for this turn + const lastUserMsg = messages[messages.length - 1]; + const stream = await chat.sendMessageStream( + model, + { message: lastUserMsg.parts ?? [] }, + 'speculation', + ); + + const modelParts: Part[] = []; + for await (const event of stream) { + if (state.abortController?.signal.aborted) break; + if (event.type !== StreamEventType.CHUNK) continue; + const response = event.value; + const parts = response.candidates?.[0]?.content?.parts ?? []; + for (const part of parts) { + // Skip thought/reasoning parts — only capture visible text + function calls + if (part.text && !(part as Record)['thought']) { + modelParts.push({ text: part.text }); + } + if (part.functionCall && part.functionCall.name) { + modelParts.push({ + functionCall: { + name: part.functionCall.name, + args: part.functionCall.args, + }, + }); + } } } - } - - if (state.abortController?.signal.aborted) break; - if (modelParts.length === 0) break; - const modelMsg: Content = { role: 'model', parts: modelParts }; - messages.push(modelMsg); - - // Extract function calls from model response - const functionCalls = modelParts.filter( - (p): p is Part & { functionCall: NonNullable } => - p.functionCall !== undefined, - ); + if (state.abortController?.signal.aborted) break; + if (modelParts.length === 0) break; - if (functionCalls.length === 0) { - // No tool calls — speculation complete (text-only response) - break; - } + const modelMsg: Content = { role: 'model', parts: modelParts }; + messages.push(modelMsg); - // Process each function call through the tool gate - const functionResponses: Part[] = []; - let hitBoundary = false; - - for (const part of functionCalls) { - const fc = part.functionCall; - const name = fc.name ?? ''; - const args = (fc.args ?? {}) as Record; - const gate = await evaluateToolCall( - name, - args, - state.overlayFs!, - approvalMode, + // Extract function calls from model response + const functionCalls = modelParts.filter( + (p): p is Part & { functionCall: NonNullable } => + p.functionCall !== undefined, ); - if (gate.action === 'boundary') { - hitBoundary = true; + if (functionCalls.length === 0) { + // No tool calls — speculation complete (text-only response) break; } - if (gate.action === 'redirect') { - try { - await rewritePathArgs(args, state.overlayFs!); - } catch { - // Path rewrite failed (e.g., absolute path outside cwd) — treat as boundary + // Process each function call through the tool gate + const functionResponses: Part[] = []; + let hitBoundary = false; + + for (const part of functionCalls) { + const fc = part.functionCall; + const name = fc.name ?? ''; + const args = (fc.args ?? {}) as Record; + const gate = await evaluateToolCall( + name, + args, + state.overlayFs!, + approvalMode, + ); + + if (gate.action === 'boundary') { hitBoundary = true; break; } - } - // Execute the tool directly (bypassing CoreToolScheduler) - // SECURITY: Only reaches here for read-only tools or writes gated by approvalMode - try { - const toolRegistry = config.getToolRegistry(); - const tool = await toolRegistry.ensureTool(name); - if (!tool) { + if (gate.action === 'redirect') { + try { + await rewritePathArgs(args, state.overlayFs!); + } catch { + // Path rewrite failed (e.g., absolute path outside cwd) — treat as boundary + hitBoundary = true; + break; + } + } + + // Execute the tool directly (bypassing CoreToolScheduler) + // SECURITY: Only reaches here for read-only tools or writes gated by approvalMode + try { + const toolRegistry = config.getToolRegistry(); + const tool = await toolRegistry.ensureTool(name); + if (!tool) { + functionResponses.push({ + functionResponse: { + name, + response: { error: `Tool '${name}' not found` }, + }, + }); + continue; + } + + const invocation = tool.build(args); + const result = await invocation.execute( + state.abortController!.signal, + ); + state.toolUseCount++; + + const responseContent = + typeof result.llmContent === 'string' + ? { output: result.llmContent } + : { output: JSON.stringify(result.llmContent) }; + functionResponses.push({ + functionResponse: { name, response: responseContent }, + }); + } catch (error: unknown) { functionResponses.push({ functionResponse: { name, - response: { error: `Tool '${name}' not found` }, + response: { + error: + error instanceof Error + ? error.message + : 'Tool execution failed', + }, }, }); - continue; } - - const invocation = tool.build(args); - const result = await invocation.execute(state.abortController!.signal); - state.toolUseCount++; - - const responseContent = - typeof result.llmContent === 'string' - ? { output: result.llmContent } - : { output: JSON.stringify(result.llmContent) }; - functionResponses.push({ - functionResponse: { name, response: responseContent }, - }); - } catch (error: unknown) { - functionResponses.push({ - functionResponse: { - name, - response: { - error: - error instanceof Error - ? error.message - : 'Tool execution failed', - }, - }, - }); } - } - if (hitBoundary) { - // Keep already-executed tool responses, strip unexecuted function calls - // from model message, and add the partial responses we do have (#18) - if (functionResponses.length > 0) { - // Some tools were executed before boundary — keep only the first N - // functionCall parts (matching functionResponses.length) by order, - // not by name, to handle duplicate tool names correctly. - let keptFunctionCalls = 0; - const keptModelParts = modelParts.filter((p) => { - if (!p.functionCall) return true; - if (keptFunctionCalls < functionResponses.length) { - keptFunctionCalls++; - return true; + if (hitBoundary) { + // Keep already-executed tool responses, strip unexecuted function calls + // from model message, and add the partial responses we do have (#18) + if (functionResponses.length > 0) { + // Some tools were executed before boundary — keep only the first N + // functionCall parts (matching functionResponses.length) by order, + // not by name, to handle duplicate tool names correctly. + let keptFunctionCalls = 0; + const keptModelParts = modelParts.filter((p) => { + if (!p.functionCall) return true; + if (keptFunctionCalls < functionResponses.length) { + keptFunctionCalls++; + return true; + } + return false; + }); + if (keptModelParts.length > 0) { + messages[messages.length - 1] = { + role: 'model', + parts: keptModelParts, + }; + // Add the tool results we have + messages.push({ role: 'user', parts: functionResponses }); + } else { + messages.pop(); } - return false; - }); - if (keptModelParts.length > 0) { - messages[messages.length - 1] = { - role: 'model', - parts: keptModelParts, - }; - // Add the tool results we have - messages.push({ role: 'user', parts: functionResponses }); } else { - messages.pop(); - } - } else { - // No tools were executed — remove the model message entirely - const textOnlyParts = modelParts.filter( - (p) => p.functionCall === undefined, - ); - if (textOnlyParts.length > 0) { - messages[messages.length - 1] = { - role: 'model', - parts: textOnlyParts, - }; - } else { - messages.pop(); + // No tools were executed — remove the model message entirely + const textOnlyParts = modelParts.filter( + (p) => p.functionCall === undefined, + ); + if (textOnlyParts.length > 0) { + messages[messages.length - 1] = { + role: 'model', + parts: textOnlyParts, + }; + } else { + messages.pop(); + } } - } - return { - messages, - boundary: { - type: 'boundary', - detail: 'speculation_boundary', - completedAt: Date.now(), - }, - }; - } + return { + messages, + boundary: { + type: 'boundary', + detail: 'speculation_boundary', + completedAt: Date.now(), + }, + }; + } - // Add tool results to history for next turn - if (functionResponses.length > 0) { - const resultMsg: Content = { role: 'user', parts: functionResponses }; - messages.push(resultMsg); + // Add tool results to history for next turn + if (functionResponses.length > 0) { + const resultMsg: Content = { role: 'user', parts: functionResponses }; + messages.push(resultMsg); + } } - } - return { messages }; + return { messages }; + }); } // --------------------------------------------------------------------------- @@ -539,12 +545,13 @@ ${SUGGESTION_PROMPT}`; const cacheSafeParams = getCacheSafeParams(); if (!cacheSafeParams) return null; + const model = modelOverride ?? config.getFastModel(); const result = await runForkedAgent({ config, userMessage: augmentedPrompt, cacheSafeParams, jsonSchema: PIPELINED_SCHEMA, - model: modelOverride, + ...(model !== undefined ? { model } : {}), abortSignal, }); diff --git a/packages/core/src/followup/suggestionGenerator.test.ts b/packages/core/src/followup/suggestionGenerator.test.ts index 38005318269..15abf5081a3 100644 --- a/packages/core/src/followup/suggestionGenerator.test.ts +++ b/packages/core/src/followup/suggestionGenerator.test.ts @@ -4,8 +4,116 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect } from 'vitest'; -import { shouldFilterSuggestion } from './suggestionGenerator.js'; +import type { Content } from '@google/genai'; +import { beforeEach, describe, it, expect, vi } from 'vitest'; +import type { Config } from '../config/config.js'; + +const { mockGetCacheSafeParams, mockRunForkedAgent, mockAddEvent } = vi.hoisted( + () => ({ + mockGetCacheSafeParams: vi.fn(), + mockRunForkedAgent: vi.fn(), + mockAddEvent: vi.fn(), + }), +); + +vi.mock('../utils/forkedAgent.js', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + getCacheSafeParams: mockGetCacheSafeParams, + runForkedAgent: mockRunForkedAgent, + }; +}); + +vi.mock('../telemetry/uiTelemetry.js', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + uiTelemetryService: { + addEvent: mockAddEvent, + }, + }; +}); + +import { + generatePromptSuggestion, + shouldFilterSuggestion, +} from './suggestionGenerator.js'; + +const conversationHistory: Content[] = [ + { role: 'user', parts: [{ text: 'fix this' }] }, + { role: 'model', parts: [{ text: 'I fixed it.' }] }, + { role: 'user', parts: [{ text: 'anything else?' }] }, + { role: 'model', parts: [{ text: 'You could run tests.' }] }, +]; + +describe('generatePromptSuggestion', () => { + beforeEach(() => { + mockGetCacheSafeParams.mockReset(); + mockRunForkedAgent.mockReset(); + mockAddEvent.mockReset(); + }); + + it('passes cache-safe model in cache mode when no explicit or fast model exists', async () => { + mockGetCacheSafeParams.mockReturnValue({ + generationConfig: {}, + history: conversationHistory, + model: 'main-model', + version: 1, + }); + mockRunForkedAgent.mockResolvedValue({ + text: null, + jsonResult: { suggestion: 'run tests' }, + usage: { inputTokens: 10, outputTokens: 3, cacheHitTokens: 5 }, + }); + const config = { + getFastModel: vi.fn(() => undefined), + getModel: vi.fn(() => 'main-model'), + } as unknown as Config; + + await generatePromptSuggestion( + config, + conversationHistory, + new AbortController().signal, + { enableCacheSharing: true }, + ); + + expect(mockRunForkedAgent).toHaveBeenCalledWith( + expect.objectContaining({ model: 'main-model' }), + ); + }); + + it('passes the fast model in cache mode when one is configured', async () => { + mockGetCacheSafeParams.mockReturnValue({ + generationConfig: {}, + history: conversationHistory, + model: 'main-model', + version: 1, + }); + mockRunForkedAgent.mockResolvedValue({ + text: null, + jsonResult: { suggestion: 'run tests' }, + usage: { inputTokens: 10, outputTokens: 3, cacheHitTokens: 5 }, + }); + const config = { + getFastModel: vi.fn(() => 'openai:fast-model'), + getModel: vi.fn(() => 'main-model'), + } as unknown as Config; + + await generatePromptSuggestion( + config, + conversationHistory, + new AbortController().signal, + { enableCacheSharing: true }, + ); + + expect(mockRunForkedAgent).toHaveBeenCalledWith( + expect.objectContaining({ model: 'openai:fast-model' }), + ); + }); +}); describe('shouldFilterSuggestion', () => { it('filters "done"', () => { diff --git a/packages/core/src/followup/suggestionGenerator.ts b/packages/core/src/followup/suggestionGenerator.ts index 4049315e14f..399533b5265 100644 --- a/packages/core/src/followup/suggestionGenerator.ts +++ b/packages/core/src/followup/suggestionGenerator.ts @@ -152,9 +152,9 @@ async function generateViaForkedQuery( abortSignal: AbortSignal, modelOverride?: string, ): Promise { - const model = modelOverride || config.getModel(); const cacheSafeParams = getCacheSafeParams(); if (!cacheSafeParams) return null; + const model = modelOverride ?? config.getFastModel() ?? cacheSafeParams.model; const startTime = Date.now(); const result = await runForkedAgent({ config, @@ -206,7 +206,7 @@ async function generateViaBaseLlm( abortSignal: AbortSignal, modelOverride?: string, ): Promise { - const model = modelOverride || config.getModel(); + const model = modelOverride ?? config.getFastModel() ?? config.getModel(); const contents: Content[] = [ ...conversationHistory, { role: 'user', parts: [{ text: SUGGESTION_PROMPT }] }, diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 5380c75c32c..2d6e7280ae2 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -928,11 +928,10 @@ export class ChatRecordingService { // Headless/one-shot CLI flows (`qwen -p "…"`, cron, CI scripts) run a // single prompt and throw the session away. Spending fast-model tokens // on a title no one will ever resume is pure waste; skip entirely. - // Checked before `getFastModelForSideQuery()` because it's strictly - // cheaper (a bool field read vs. a method that looks up available models). + // Checked before `getFastModel()` because it's strictly cheaper (a bool + // field read vs. a method that looks up available models). if (!this.config.isInteractive()) return; - const fastModel = - this.config.getFastModelForSideQuery?.() ?? this.config.getFastModel(); + const fastModel = this.config.getFastModel(); if (!fastModel) return; this.autoTitleAttempts++; diff --git a/packages/core/src/services/sessionRecap.ts b/packages/core/src/services/sessionRecap.ts index de66aae3296..4f0ba75da60 100644 --- a/packages/core/src/services/sessionRecap.ts +++ b/packages/core/src/services/sessionRecap.ts @@ -32,11 +32,6 @@ const RECAP_USER_PROMPT = const RECAP_OPEN_TAG = ''; const RECAP_TAG_RE = /([\s\S]*?)<\/recap>/i; -export interface SessionRecapResult { - text: string; - modelUsed: string; -} - /** * Generate a 1-2 sentence "where did I leave off" summary of the current * session. Uses the configured fast model (falls back to main model) with @@ -49,7 +44,7 @@ export interface SessionRecapResult { export async function generateSessionRecap( config: Config, abortSignal: AbortSignal, -): Promise { +): Promise { try { const geminiClient = config.getGeminiClient(); if (!geminiClient) return null; @@ -61,11 +56,6 @@ export async function generateSessionRecap( const recentHistory = takeRecentDialog(dialog, RECENT_MESSAGE_WINDOW); if (recentHistory.length === 0) return null; - const model = - config.getFastModelForSideQuery?.() ?? - config.getFastModel() ?? - config.getModel(); - const result = await runSideQuery(config, { purpose: 'session-recap', contents: [ @@ -89,7 +79,7 @@ export async function generateSessionRecap( const text = extractRecap(result.text); if (!text) return null; - return { text, modelUsed: model }; + return text; } catch (err) { debugLogger.warn( `Recap generation failed: ${err instanceof Error ? err.message : String(err)}`, diff --git a/packages/core/src/services/sessionTitle.ts b/packages/core/src/services/sessionTitle.ts index 8e3e4124465..331b5e86361 100644 --- a/packages/core/src/services/sessionTitle.ts +++ b/packages/core/src/services/sessionTitle.ts @@ -70,7 +70,7 @@ const TRAILING_PAIRED_BRACKETS_RE = * command) can surface actionable messages instead of a generic "could not * generate". * - * - `no_fast_model`: config.getFastModelForSideQuery() returned undefined. + * - `no_fast_model`: config.getFastModel() returned undefined. * User needs to configure one via `/model --fast `. * - `no_client`: BaseLlmClient or GeminiClient not yet initialized. Rare, * usually means the session hasn't authenticated yet. @@ -107,7 +107,7 @@ export async function tryGenerateSessionTitle( abortSignal: AbortSignal, ): Promise { try { - const model = config.getFastModelForSideQuery?.() ?? config.getFastModel(); + const model = config.getFastModel(); if (!model) return { ok: false, reason: 'no_fast_model' }; const geminiClient = config.getGeminiClient(); diff --git a/packages/core/src/services/toolUseSummary.test.ts b/packages/core/src/services/toolUseSummary.test.ts index b9633d4a124..69d77311a78 100644 --- a/packages/core/src/services/toolUseSummary.test.ts +++ b/packages/core/src/services/toolUseSummary.test.ts @@ -310,25 +310,6 @@ describe('generateToolUseSummary', () => { expect(userText).not.toContain('A'.repeat(201)); }); - it('uses explicit model parameter over config fast model', async () => { - const generateContentFn = vi.fn().mockResolvedValue({ - text: 'Done', - usage: undefined, - }); - const config = makeMockConfig('qwen-fast', generateContentFn); - - await generateToolUseSummary({ - config, - tools: [{ name: 'Edit', input: {}, output: '' }], - signal: abortController().signal, - model: 'qwen-turbo-explicit', - }); - - expect(generateContentFn.mock.calls[0][0].model).toBe( - 'qwen-turbo-explicit', - ); - }); - it('returns null when model returns empty text', async () => { const generateContentFn = vi.fn().mockResolvedValue({ text: '', diff --git a/packages/core/src/services/toolUseSummary.ts b/packages/core/src/services/toolUseSummary.ts index 6ad4987a8f6..f26a8534729 100644 --- a/packages/core/src/services/toolUseSummary.ts +++ b/packages/core/src/services/toolUseSummary.ts @@ -88,14 +88,6 @@ export interface GenerateToolUseSummaryParams { * so the summarizer knows what the user was trying to accomplish. */ lastAssistantText?: string; - /** - * Fast model to use. If omitted, falls back to - * `config.getFastModelForSideQuery()`; - * if that also returns undefined, the call is skipped (returns null). - * Unlike `sessionRecap`, this does not fall back to the main model — - * summary generation is a nice-to-have and must not incur main-model cost. - */ - model?: string; } /** @@ -113,10 +105,7 @@ export async function generateToolUseSummary( return null; } - const model = - params.model ?? - config.getFastModelForSideQuery?.() ?? - config.getFastModel(); + const model = config.getFastModel(); if (!model) { debugLogger.debug('No fast model configured — skipping summary generation'); return null; @@ -155,7 +144,7 @@ export async function generateToolUseSummary( temperature: 0.3, }, abortSignal: signal, - ...(params.model !== undefined ? { model: params.model } : {}), + model, // Tool-use labels are best-effort cosmetic; firing once per turn means // 7 retries on a transient outage would spike traffic for no benefit. maxAttempts: 1, diff --git a/packages/core/src/skills/types.ts b/packages/core/src/skills/types.ts index 206c711edab..94f4d4df593 100644 --- a/packages/core/src/skills/types.ts +++ b/packages/core/src/skills/types.ts @@ -50,8 +50,9 @@ export interface SkillConfig { /** * Optional model override for this skill's execution. * Uses the same selector syntax as subagent model selectors: - * bare model ID (e.g., `qwen-coder-plus`), `authType:modelId` - * for cross-provider, or omitted/`inherit` to use the session model. + * `fast`, bare model ID (e.g., `qwen-coder-plus`), + * `authType:modelId` for cross-provider, or omitted/`inherit` + * to use the session model. */ model?: string; diff --git a/packages/core/src/subagents/model-selection.test.ts b/packages/core/src/subagents/model-selection.test.ts deleted file mode 100644 index dfee6c57593..00000000000 --- a/packages/core/src/subagents/model-selection.test.ts +++ /dev/null @@ -1,77 +0,0 @@ -/** - * @license - * Copyright 2025 Qwen - * SPDX-License-Identifier: Apache-2.0 - */ - -import { describe, expect, it } from 'vitest'; -import { AuthType } from '../core/contentGenerator.js'; -import { parseSubagentModelSelection } from './model-selection.js'; - -describe('parseSubagentModelSelection', () => { - it('treats omitted models as inherit', () => { - expect(parseSubagentModelSelection(undefined)).toEqual({ - inherits: true, - }); - }); - - it('treats explicit inherit as inherit', () => { - expect(parseSubagentModelSelection('inherit')).toEqual({ - inherits: true, - }); - }); - - it('parses bare model IDs', () => { - expect(parseSubagentModelSelection('glm-5')).toEqual({ - modelId: 'glm-5', - inherits: false, - }); - }); - - it('parses authType-prefixed model IDs', () => { - expect(parseSubagentModelSelection('openai:glm-5')).toEqual({ - authType: AuthType.USE_OPENAI, - modelId: 'glm-5', - inherits: false, - }); - }); - - it('treats unknown prefix as bare model ID (colon in model ID)', () => { - expect(parseSubagentModelSelection('invalid:glm-5')).toEqual({ - modelId: 'invalid:glm-5', - inherits: false, - }); - }); - - it('treats model IDs with colons as bare model IDs', () => { - expect(parseSubagentModelSelection('gpt-4o:online')).toEqual({ - modelId: 'gpt-4o:online', - inherits: false, - }); - }); - - it('parses the fast keyword', () => { - expect(parseSubagentModelSelection('fast')).toEqual({ - inherits: false, - usesFastModel: true, - }); - }); - - it('parses the fast keyword with surrounding whitespace', () => { - expect(parseSubagentModelSelection(' fast ')).toEqual({ - inherits: false, - usesFastModel: true, - }); - }); - - it('treats model IDs that merely contain "fast" as bare IDs, not the keyword', () => { - expect(parseSubagentModelSelection('qwen3-coder-flash')).toEqual({ - modelId: 'qwen3-coder-flash', - inherits: false, - }); - expect(parseSubagentModelSelection('Fast')).toEqual({ - modelId: 'Fast', - inherits: false, - }); - }); -}); diff --git a/packages/core/src/subagents/model-selection.ts b/packages/core/src/subagents/model-selection.ts deleted file mode 100644 index 8cd540a477b..00000000000 --- a/packages/core/src/subagents/model-selection.ts +++ /dev/null @@ -1,69 +0,0 @@ -/** - * @license - * Copyright 2025 Qwen - * SPDX-License-Identifier: Apache-2.0 - */ - -import { AuthType } from '../core/contentGenerator.js'; - -export interface ParsedSubagentModelSelection { - authType?: AuthType; - modelId?: string; - inherits: boolean; - /** - * True when the selector was `fast` — the runtime resolves this to - * `Config.getFastModel()` if a valid fast model is configured, and - * falls back to inheriting the parent model otherwise. - */ - usesFastModel?: boolean; -} - -const AUTH_TYPES = new Set(Object.values(AuthType)); - -/** - * Parse a subagent model selector. - * - * Supported forms: - * - omitted / inherit -> use parent conversation model - * - fast -> use Config.getFastModel() if available, else inherit parent model - * - modelId -> use parent authType with the provided modelId - * - authType:modelId -> use explicit authType and modelId - */ -export function parseSubagentModelSelection( - model: string | undefined, -): ParsedSubagentModelSelection { - const trimmed = model?.trim(); - if (!trimmed || trimmed === 'inherit') { - return { inherits: true }; - } - - if (trimmed === 'fast') { - return { inherits: false, usesFastModel: true }; - } - - const colonIndex = trimmed.indexOf(':'); - if (colonIndex === -1) { - return { modelId: trimmed, inherits: false }; - } - - const maybeAuthType = trimmed.slice(0, colonIndex).trim(); - const modelId = trimmed.slice(colonIndex + 1).trim(); - - // If the prefix isn't a known AuthType, treat the whole string as a bare - // model ID. Model IDs can legitimately contain colons (e.g. gpt-4o:online). - if (!AUTH_TYPES.has(maybeAuthType as AuthType)) { - return { modelId: trimmed, inherits: false }; - } - - if (!modelId) { - throw new Error( - 'Model selector must include a model ID after the authType', - ); - } - - return { - authType: maybeAuthType as AuthType, - modelId, - inherits: false, - }; -} diff --git a/packages/core/src/subagents/subagent-manager.test.ts b/packages/core/src/subagents/subagent-manager.test.ts index ea64aeb1431..e6030713748 100644 --- a/packages/core/src/subagents/subagent-manager.test.ts +++ b/packages/core/src/subagents/subagent-manager.test.ts @@ -1398,7 +1398,7 @@ System prompt 3`); expect(runtimeConfig.modelConfig.model).toBe('gpt-4'); }); - it('should resolve "fast" to Config.getFastModel() when one is configured', async () => { + it('should resolve "fast" to the configured current-auth fast model', async () => { const fastConfig: SubagentConfig = { ...validConfig, model: 'fast' }; vi.spyOn(mockConfig, 'getFastModel').mockReturnValue('fast-model-id'); @@ -1410,6 +1410,20 @@ System prompt 3`); expect(runtimeConfig.modelConfig.model).toBe('fast-model-id'); }); + it('should resolve "fast" to authType-qualified fast model selectors', async () => { + const fastConfig: SubagentConfig = { ...validConfig, model: 'fast' }; + vi.spyOn(mockConfig, 'getFastModel').mockReturnValue( + 'openai:fast-model-id', + ); + + const runtimeConfig = await manager.convertToRuntimeConfig( + fastConfig, + mockConfig, + ); + + expect(runtimeConfig.modelConfig.model).toBe('fast-model-id'); + }); + it('should leave modelConfig empty for "fast" when getFastModel returns undefined', async () => { // Mirrors the unset / invalid-for-authType cases — AgentCore then // falls back to runtimeContext.getModel() (the parent model). @@ -1575,6 +1589,62 @@ System prompt 3`); ); }); + it('should build a cross-auth ContentGenerator when "fast" resolves to an authType-qualified selector', async () => { + const config = { ...agentConfig, model: 'fast' }; + vi.spyOn(mockConfig, 'getContentGeneratorConfig').mockReturnValue({ + model: 'parent-model', + authType: AuthType.USE_ANTHROPIC, + apiKey: 'parent-key', + }); + vi.spyOn(mockConfig, 'getFastModel').mockReturnValue( + 'openai:deepseek-v4-flash', + ); + + await manager.createAgentHeadless(config, mockConfig); + + expect(mockCreateContentGenerator).toHaveBeenCalledWith( + expect.objectContaining({ + model: 'deepseek-v4-flash', + authType: AuthType.USE_OPENAI, + }), + mockConfig, + ); + }); + + it('should resolve bare fast models to their configured auth type when current auth does not own them', async () => { + const config = { ...agentConfig, model: 'fast' }; + vi.spyOn(mockConfig, 'getContentGeneratorConfig').mockReturnValue({ + model: 'claude-opus', + authType: AuthType.USE_ANTHROPIC, + apiKey: 'parent-key', + }); + vi.spyOn(mockConfig, 'getFastModel').mockReturnValue( + 'deepseek-v4-flash', + ); + vi.spyOn(mockConfig, 'getAllConfiguredModels').mockImplementation( + (authTypes) => + authTypes?.includes(AuthType.USE_ANTHROPIC) + ? [] + : [ + { + id: 'deepseek-v4-flash', + label: 'deepseek-v4-flash', + authType: AuthType.USE_OPENAI, + }, + ], + ); + + await manager.createAgentHeadless(config, mockConfig); + + expect(mockCreateContentGenerator).toHaveBeenCalledWith( + expect.objectContaining({ + model: 'deepseek-v4-flash', + authType: AuthType.USE_OPENAI, + }), + mockConfig, + ); + }); + it('should NOT build a new ContentGenerator for "fast" when getFastModel returns undefined', async () => { const config = { ...agentConfig, model: 'fast' }; vi.spyOn(mockConfig, 'getFastModel').mockReturnValue(undefined); diff --git a/packages/core/src/subagents/subagent-manager.ts b/packages/core/src/subagents/subagent-manager.ts index b58e27697f7..f5564b4c210 100644 --- a/packages/core/src/subagents/subagent-manager.ts +++ b/packages/core/src/subagents/subagent-manager.ts @@ -39,7 +39,11 @@ import type { RuntimeContentGeneratorView } from '../agents/runtime/agent-contex import { createRuntimeContentGeneratorView } from '../models/content-generator-config.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { normalizeContent } from '../utils/textUtils.js'; -import { parseSubagentModelSelection } from './model-selection.js'; +import { + buildModelIdContext, + resolveModelId, + type ResolvedModelId, +} from '../utils/modelId.js'; const debugLogger = createDebugLogger('SUBAGENT_MANAGER'); import { BuiltinAgentRegistry } from './builtin-agents.js'; import { ToolDisplayNamesMigration } from '../tools/tool-names.js'; @@ -725,10 +729,11 @@ export class SubagentManager { } /** - * When a subagent's model selector specifies a model (bare ID or - * authType-prefixed), build a dedicated ContentGenerator and the view - * the agent runtime should publish via AsyncLocalStorage during the - * run. Returns `undefined` for inherit selectors (no override needed). + * When a subagent's model selector resolves to a concrete model, build a + * dedicated ContentGenerator and the view the agent runtime should publish + * via AsyncLocalStorage during the run. Returns `undefined` when no + * override is needed — including `inherit`, an unset `fast` selector, or + * any selector that fails to resolve to a configured model. * * FileReadCache isolation and tool-registry rebuilding are handled * separately in {@link buildSubagentContextOverride} — every subagent @@ -739,27 +744,13 @@ export class SubagentManager { config: SubagentConfig, base: Config, ): Promise { - const selection = parseSubagentModelSelection(config.model); - if (selection.inherits) { + const resolvedModel = this.resolveModelOverride(config.model, base); + if (!resolvedModel) { return undefined; } - let resolvedModelId = selection.modelId; - if (selection.usesFastModel) { - // getFastModel() returns the fastModel id only when it's valid for - // the current authType; otherwise undefined. Treat undefined as - // inherit so an unset or invalid fastModel silently falls back to - // the parent session model — matching every other getFastModel() - // call site (ForkedAgent, sessionTitle, etc.). - const fast = base.getFastModel(); - if (!fast) { - return undefined; - } - resolvedModelId = fast; - } - const authType = - selection.authType ?? base.getContentGeneratorConfig().authType; + resolvedModel.authType ?? base.getContentGeneratorConfig().authType; const authOverrides = { authType: authType as string, }; @@ -767,7 +758,7 @@ export class SubagentManager { const view = await createRuntimeContentGeneratorView( base, base, - resolvedModelId, + resolvedModel.modelId, authOverrides, ); @@ -778,6 +769,17 @@ export class SubagentManager { return view; } + private resolveModelOverride( + model: string | undefined, + runtimeContext?: Config, + ): ResolvedModelId | undefined { + // Omit currentModel so `inherit` resolves to undefined; subagents treat + // "inherit / no override" as a signal to skip building a dedicated + // ContentGenerator entirely. + const context = runtimeContext ? buildModelIdContext(runtimeContext) : {}; + return resolveModelId(model, { ...context, currentModel: undefined }); + } + /** * Converts a file-based SubagentConfig to runtime configuration * compatible with AgentHeadless.create(). @@ -793,16 +795,12 @@ export class SubagentManager { systemPrompt: config.systemPrompt, }; - const selection = parseSubagentModelSelection(config.model); - let resolvedModelId = selection.modelId; - if (selection.usesFastModel && runtimeContext) { - // Resolve `fast` to the configured fastModel. Undefined here means - // either unset or invalid for the current authType — leave modelConfig - // empty so the agent inherits the parent model (same as `inherit`). - resolvedModelId = runtimeContext.getFastModel(); - } + const resolvedModel = this.resolveModelOverride( + config.model, + runtimeContext, + ); const modelConfig: ModelConfig = { - ...(resolvedModelId ? { model: resolvedModelId } : {}), + ...(resolvedModel ? { model: resolvedModel.modelId } : {}), }; const runConfig: RunConfig = { diff --git a/packages/core/src/subagents/types.ts b/packages/core/src/subagents/types.ts index 216390520eb..18c8f96ef95 100644 --- a/packages/core/src/subagents/types.ts +++ b/packages/core/src/subagents/types.ts @@ -83,8 +83,8 @@ export interface SubagentConfig { /** * Optional model selector. * - Omitted or 'inherit': use the main conversation model - * - 'fast': use Config.getFastModel() under the parent authType when - * configured and valid; silently inherit the parent model otherwise + * - 'fast': use the configured fast model when available; supports + * authType-qualified fastModel settings and silently inherits otherwise * - 'model-id': use the given model with the main conversation authType * - 'authType:model-id': use the given authType and model ID */ diff --git a/packages/core/src/subagents/validation.ts b/packages/core/src/subagents/validation.ts index 9f05b0d60b2..c43935841ea 100644 --- a/packages/core/src/subagents/validation.ts +++ b/packages/core/src/subagents/validation.ts @@ -7,7 +7,7 @@ import { SubagentError, SubagentErrorCode } from './types.js'; import type { SubagentConfig, ValidationResult } from './types.js'; import type { RunConfig } from '../agents/runtime/agent-types.js'; -import { parseSubagentModelSelection } from './model-selection.js'; +import { resolveModelId } from '../utils/modelId.js'; /** * Validates subagent configurations to ensure they are well-formed @@ -276,7 +276,7 @@ export class SubagentValidator { } try { - parseSubagentModelSelection(model); + resolveModelId(model); } catch (error) { errors.push(error instanceof Error ? error.message : 'Invalid model'); } diff --git a/packages/core/src/tools/skill.test.ts b/packages/core/src/tools/skill.test.ts index e3ba0dec933..df394d8385f 100644 --- a/packages/core/src/tools/skill.test.ts +++ b/packages/core/src/tools/skill.test.ts @@ -790,22 +790,25 @@ describe('SkillTool', () => { }); describe('modelOverride propagation', () => { - it('should propagate model from skill config to ToolResult', async () => { - const skillWithModel: SkillConfig = { - ...mockSkills[0], - model: 'qwen-max', - }; - vi.mocked(mockSkillManager.loadSkillForRuntime).mockResolvedValue( - skillWithModel, - ); - - const invocation = ( - skillTool as SkillToolWithProtectedMethods - ).createInvocation({ skill: 'code-review' }); - const result = (await invocation.execute()) as unknown as ToolResult; - - expect(result.modelOverride).toBe('qwen-max'); - }); + it.each(['qwen-max', 'fast', 'openai:qwen-max'])( + 'should propagate model selector "%s" from skill config to ToolResult', + async (model) => { + const skillWithModel: SkillConfig = { + ...mockSkills[0], + model, + }; + vi.mocked(mockSkillManager.loadSkillForRuntime).mockResolvedValue( + skillWithModel, + ); + + const invocation = ( + skillTool as SkillToolWithProtectedMethods + ).createInvocation({ skill: 'code-review' }); + const result = (await invocation.execute()) as unknown as ToolResult; + + expect(result.modelOverride).toBe(model); + }, + ); it('should set modelOverride to undefined when skill has no model', async () => { const skillWithoutModel: SkillConfig = { diff --git a/packages/core/src/utils/forkedAgent.agent.test.ts b/packages/core/src/utils/forkedAgent.agent.test.ts index f92c428564a..7e5af4dd281 100644 --- a/packages/core/src/utils/forkedAgent.agent.test.ts +++ b/packages/core/src/utils/forkedAgent.agent.test.ts @@ -4,11 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import type { Config } from '../config/config.js'; import { Config as ConfigImpl, ApprovalMode } from '../config/config.js'; import { AgentHeadless } from '../agents/runtime/agent-headless.js'; import { AgentTerminateMode } from '../agents/runtime/agent-types.js'; +import type { ModelConfig } from '../agents/runtime/agent-types.js'; import { runForkedAgent } from './forkedAgent.js'; import { ToolNames } from '../tools/tool-names.js'; import { EditTool } from '../tools/edit.js'; @@ -16,6 +17,30 @@ import { hasRebuiltToolRegistry, TOOL_REGISTRY_REBUILT, } from '../tools/agent/agent.js'; +import { AuthType } from '../core/contentGenerator.js'; +import type { RuntimeContentGeneratorView } from '../agents/runtime/agent-context.js'; +import { createRuntimeContentGeneratorView } from '../models/content-generator-config.js'; + +vi.mock('../models/content-generator-config.js', async (importOriginal) => { + const actual = + await importOriginal< + typeof import('../models/content-generator-config.js') + >(); + return { + ...actual, + createRuntimeContentGeneratorView: vi.fn(), + }; +}); + +function makeRuntimeView(model: string): RuntimeContentGeneratorView { + return { + contentGenerator: {} as RuntimeContentGeneratorView['contentGenerator'], + contentGeneratorConfig: { + model, + authType: AuthType.USE_OPENAI, + }, + }; +} /** * Regression: `runForkedAgent` (AgentHeadless path) used to produce its @@ -31,6 +56,10 @@ import { * to the wrapper. */ describe('runForkedAgent (AgentHeadless path) bound-tool isolation', () => { + beforeEach(() => { + vi.mocked(createRuntimeContentGeneratorView).mockReset(); + }); + // Bare mode keeps the registry small (ReadFile / Edit / Shell only) so // the rebuild covers the file tools we actually care about. const baseParams = { @@ -322,4 +351,92 @@ describe('runForkedAgent (AgentHeadless path) bound-tool isolation', () => { await new Promise((resolve) => setImmediate(resolve)); expect(stopSpy).toHaveBeenCalledTimes(1); }); + + it('uses a runtime content-generator view for cross-auth fast models', async () => { + const fastModel = 'deepseek-v4-flash'; + const runtimeView = makeRuntimeView(fastModel); + vi.mocked(createRuntimeContentGeneratorView).mockResolvedValue(runtimeView); + + const parent = new ConfigImpl({ + ...baseParams, + model: 'claude-main', + }); + const parentRegistry = await parent.createToolRegistry(undefined, { + skipDiscovery: true, + }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (parent as any).toolRegistry = parentRegistry; + + vi.spyOn(parent, 'getContentGeneratorConfig').mockReturnValue({ + model: 'claude-main', + authType: AuthType.USE_ANTHROPIC, + }); + vi.spyOn(parent, 'getFastModel').mockReturnValue( + `${AuthType.USE_OPENAI}:${fastModel}`, + ); + vi.spyOn(parent, 'getAllConfiguredModels').mockImplementation( + (authTypes?: AuthType[]) => + authTypes?.includes(AuthType.USE_OPENAI) + ? [ + { + id: fastModel, + label: fastModel, + authType: AuthType.USE_OPENAI, + }, + ] + : [], + ); + + const captured: { + config?: Config; + modelConfig?: ModelConfig; + runtimeView?: RuntimeContentGeneratorView; + } = {}; + const createSpy = vi + .spyOn(AgentHeadless, 'create') + .mockImplementation( + async ( + _name: string, + config: Config, + _promptConfig: unknown, + modelConfig: ModelConfig, + _runConfig: unknown, + _toolConfig: unknown, + _eventEmitter: unknown, + _hooks: unknown, + runtimeViewArg?: RuntimeContentGeneratorView, + ): Promise => { + captured.config = config; + captured.modelConfig = modelConfig; + captured.runtimeView = runtimeViewArg; + return { + execute: vi.fn().mockResolvedValue(undefined), + getTerminateMode: vi.fn().mockReturnValue(AgentTerminateMode.GOAL), + getFinalText: vi.fn().mockReturnValue('done'), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + }, + ); + + try { + const result = await runForkedAgent({ + name: 'test-fork', + systemPrompt: 'You are a test fork.', + taskPrompt: 'do the task', + config: parent, + }); + expect(result.status).toBe('completed'); + } finally { + createSpy.mockRestore(); + } + + expect(captured.modelConfig?.model).toBe(fastModel); + expect(captured.runtimeView).toBe(runtimeView); + expect(createRuntimeContentGeneratorView).toHaveBeenCalledWith( + parent, + captured.config, + fastModel, + { authType: AuthType.USE_OPENAI }, + ); + }); }); diff --git a/packages/core/src/utils/forkedAgent.cache.test.ts b/packages/core/src/utils/forkedAgent.cache.test.ts index 59b448c12c6..1f1fb6c2ac6 100644 --- a/packages/core/src/utils/forkedAgent.cache.test.ts +++ b/packages/core/src/utils/forkedAgent.cache.test.ts @@ -13,7 +13,10 @@ import { } from './forkedAgent.js'; import type { GenerateContentConfig } from '@google/genai'; import type { Config } from '../config/config.js'; +import { AuthType } from '../core/contentGenerator.js'; import { GeminiChat, StreamEventType } from '../core/geminiChat.js'; +import { createRuntimeContentGeneratorView } from '../models/content-generator-config.js'; +import type { RuntimeContentGeneratorView } from '../agents/runtime/agent-context.js'; vi.mock('../core/geminiChat.js', async (importOriginal) => { const actual = await importOriginal(); @@ -23,6 +26,27 @@ vi.mock('../core/geminiChat.js', async (importOriginal) => { }; }); +vi.mock('../models/content-generator-config.js', async (importOriginal) => { + const actual = + await importOriginal< + typeof import('../models/content-generator-config.js') + >(); + return { + ...actual, + createRuntimeContentGeneratorView: vi.fn(), + }; +}); + +function makeRuntimeView(model: string): RuntimeContentGeneratorView { + return { + contentGenerator: {} as RuntimeContentGeneratorView['contentGenerator'], + contentGeneratorConfig: { + model, + authType: AuthType.USE_OPENAI, + }, + }; +} + describe('CacheSafeParams', () => { beforeEach(() => { clearCacheSafeParams(); @@ -129,6 +153,7 @@ describe('runForkedAgent (cache path)', () => { beforeEach(() => { clearCacheSafeParams(); vi.mocked(GeminiChat).mockReset(); + vi.mocked(createRuntimeContentGeneratorView).mockReset(); }); it('passes tools: [] in per-request config so the model cannot produce function calls', async () => { @@ -310,6 +335,245 @@ describe('runForkedAgent (cache path)', () => { expect(result.jsonResult).toEqual({ suggestion: 'run tests' }); }); + it('routes a cross-auth fast model through a runtime content-generator view', async () => { + const fastModel = 'deepseek-v4-flash'; + const runtimeView = makeRuntimeView(fastModel); + vi.mocked(createRuntimeContentGeneratorView).mockResolvedValue(runtimeView); + + saveCacheSafeParams( + { + systemInstruction: 'You are helpful', + }, + [{ role: 'user', parts: [{ text: 'hello' }] }], + 'claude-main', + ); + + const mockSendMessageStream = vi.fn( + (_model: string, _params: unknown, _promptId: string) => { + async function* generate() { + yield { + type: StreamEventType.CHUNK, + value: { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: 'commit this' }], + }, + }, + ], + }, + }; + } + return Promise.resolve(generate()); + }, + ); + + vi.mocked(GeminiChat).mockImplementation( + () => + ({ + sendMessageStream: mockSendMessageStream, + }) as unknown as GeminiChat, + ); + + const mockConfig = { + getModel: vi.fn().mockReturnValue('claude-main'), + getContentGeneratorConfig: vi.fn().mockReturnValue({ + model: 'claude-main', + authType: AuthType.USE_ANTHROPIC, + }), + getFastModel: vi + .fn() + .mockReturnValue(`${AuthType.USE_OPENAI}:${fastModel}`), + getAllConfiguredModels: vi.fn((authTypes?: AuthType[]) => + authTypes?.includes(AuthType.USE_OPENAI) + ? [ + { + id: fastModel, + label: fastModel, + authType: AuthType.USE_OPENAI, + }, + ] + : [], + ), + } as unknown as Config; + + const result = await runForkedAgent({ + config: mockConfig, + userMessage: 'suggest something', + cacheSafeParams: getCacheSafeParams()!, + model: 'fast', + }); + + expect(result.text).toBe('commit this'); + expect(createRuntimeContentGeneratorView).toHaveBeenCalledWith( + mockConfig, + mockConfig, + fastModel, + { authType: AuthType.USE_OPENAI }, + ); + expect(mockSendMessageStream).toHaveBeenCalledWith( + fastModel, + expect.objectContaining({ + message: [{ text: 'suggest something' }], + }), + 'forked_query', + ); + }); + + it('routes a same-auth fast model through a runtime content-generator view when the model differs', async () => { + const fastModel = 'deepseek-v4-flash'; + const runtimeView = makeRuntimeView(fastModel); + vi.mocked(createRuntimeContentGeneratorView).mockResolvedValue(runtimeView); + + saveCacheSafeParams( + { + systemInstruction: 'You are helpful', + }, + [{ role: 'user', parts: [{ text: 'hello' }] }], + 'gpt-4', + ); + + const mockSendMessageStream = vi.fn( + (_model: string, _params: unknown, _promptId: string) => { + async function* generate() { + yield { + type: StreamEventType.CHUNK, + value: { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: 'commit this' }], + }, + }, + ], + }, + }; + } + return Promise.resolve(generate()); + }, + ); + + vi.mocked(GeminiChat).mockImplementation( + () => + ({ + sendMessageStream: mockSendMessageStream, + }) as unknown as GeminiChat, + ); + + const mockConfig = { + getModel: vi.fn().mockReturnValue('gpt-4'), + getContentGeneratorConfig: vi.fn().mockReturnValue({ + model: 'gpt-4', + authType: AuthType.USE_OPENAI, + }), + getFastModel: vi + .fn() + .mockReturnValue(`${AuthType.USE_OPENAI}:${fastModel}`), + getAllConfiguredModels: vi.fn((authTypes?: AuthType[]) => + authTypes?.includes(AuthType.USE_OPENAI) + ? [ + { + id: 'gpt-4', + label: 'gpt-4', + authType: AuthType.USE_OPENAI, + }, + { + id: fastModel, + label: fastModel, + authType: AuthType.USE_OPENAI, + }, + ] + : [], + ), + } as unknown as Config; + + const result = await runForkedAgent({ + config: mockConfig, + userMessage: 'suggest something', + cacheSafeParams: getCacheSafeParams()!, + model: 'fast', + }); + + expect(result.text).toBe('commit this'); + expect(createRuntimeContentGeneratorView).toHaveBeenCalledWith( + mockConfig, + mockConfig, + fastModel, + { authType: AuthType.USE_OPENAI }, + ); + expect(mockSendMessageStream).toHaveBeenCalledWith( + fastModel, + expect.objectContaining({ + message: [{ text: 'suggest something' }], + }), + 'forked_query', + ); + }); + + it('falls back to the parent model when `fast` cannot resolve (no fast model configured)', async () => { + // Public API footgun: a caller passing `model: 'fast'` while the user + // has no fast model configured must not see the literal `'fast'` sent + // to the provider. The forked path should inherit the parent model in + // that case, matching the subagent path's semantics. + saveCacheSafeParams( + { systemInstruction: 'You are helpful' }, + [{ role: 'user', parts: [{ text: 'hello' }] }], + 'parent-model', + ); + + let capturedModel: string | undefined; + const mockSendMessageStream = vi.fn( + (model: string, _params: unknown, _promptId: string) => { + capturedModel = model; + async function* generate() { + yield { + type: StreamEventType.CHUNK, + value: { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: 'ok' }], + }, + }, + ], + }, + }; + } + return Promise.resolve(generate()); + }, + ); + + vi.mocked(GeminiChat).mockImplementation( + () => + ({ + sendMessageStream: mockSendMessageStream, + }) as unknown as GeminiChat, + ); + + const mockConfig = { + getModel: vi.fn().mockReturnValue('parent-model'), + getContentGeneratorConfig: vi.fn().mockReturnValue({ + model: 'parent-model', + authType: AuthType.QWEN_OAUTH, + }), + getFastModel: vi.fn().mockReturnValue(undefined), + getAllConfiguredModels: vi.fn(() => []), + } as unknown as Config; + + await runForkedAgent({ + config: mockConfig, + userMessage: 'suggest something', + cacheSafeParams: getCacheSafeParams()!, + model: 'fast', + }); + + expect(capturedModel).toBe('parent-model'); + expect(createRuntimeContentGeneratorView).not.toHaveBeenCalled(); + }); + it('throws when CacheSafeParams are not available', async () => { const mockConfig = {} as unknown as Config; diff --git a/packages/core/src/utils/forkedAgent.ts b/packages/core/src/utils/forkedAgent.ts index 162a51f1a11..14b4dd6a8ca 100644 --- a/packages/core/src/utils/forkedAgent.ts +++ b/packages/core/src/utils/forkedAgent.ts @@ -29,8 +29,13 @@ import type { GenerateContentConfig, GenerateContentResponseUsageMetadata, } from '@google/genai'; +import { + runWithRuntimeContentGenerator, + type RuntimeContentGeneratorView, +} from '../agents/runtime/agent-context.js'; import { ApprovalMode, type Config } from '../config/config.js'; import { GeminiChat, StreamEventType } from '../core/geminiChat.js'; +import { createRuntimeContentGeneratorView } from '../models/content-generator-config.js'; import { createApprovalModeOverride } from '../tools/agent/agent.js'; import { AgentHeadless, @@ -43,6 +48,11 @@ import { type RunConfig, type ToolConfig, } from '../agents/index.js'; +import { + buildModelIdContext, + resolveModelId, + type ResolvedModelId, +} from './modelId.js'; // --------------------------------------------------------------------------- // CacheSafeParams — shared prompt-cache slot @@ -155,6 +165,85 @@ export function createForkedChat( ); } +interface ForkedModelRuntime { + model: string; + runtimeView?: RuntimeContentGeneratorView; +} + +async function buildForkedModelRuntime( + base: Config, + contentGeneratorOwner: Config, + modelSelector: string, +): Promise { + const resolvedModel = resolveModelId( + modelSelector, + buildModelIdContext(base), + ); + // When the selector cannot resolve (e.g. `fast` with no fast model + // configured, or `inherit` on a config without a current model), fall back + // to the parent session model instead of passing the raw selector string + // to the provider. Matches the subagent path, where an unresolvable + // selector means "inherit parent". + const model = resolvedModel?.modelId ?? base.getModel(); + const runtimeView = await buildForkedRuntimeContentGeneratorView( + base, + contentGeneratorOwner, + resolvedModel, + ); + + return { model, runtimeView }; +} + +async function buildForkedRuntimeContentGeneratorView( + base: Config, + contentGeneratorOwner: Config, + resolvedModel: ResolvedModelId | undefined, +): Promise { + if (!resolvedModel?.authType) return undefined; + + const currentContentGeneratorConfig = base.getContentGeneratorConfig?.(); + const currentAuthType = currentContentGeneratorConfig?.authType; + const currentModel = + currentContentGeneratorConfig?.model ?? base.getModel?.(); + if ( + resolvedModel.authType === currentAuthType && + resolvedModel.modelId === currentModel + ) { + return undefined; + } + + return createRuntimeContentGeneratorView( + base, + contentGeneratorOwner, + resolvedModel.modelId, + { authType: resolvedModel.authType }, + ); +} + +function runWithForkedModelRuntime( + runtime: ForkedModelRuntime, + fn: (model: string) => Promise, +): Promise { + const run = () => fn(runtime.model); + return runtime.runtimeView + ? runWithRuntimeContentGenerator(runtime.runtimeView, run) + : run(); +} + +/** + * Run a direct forked-chat loop under the runtime view required by the + * selected model. This is used by speculation, which owns its own multi-turn + * loop instead of going through runForkedAgent(). + */ +export async function runWithForkedChatModel( + config: Config, + modelSelector: string, + fn: (model: string) => Promise, +): Promise { + const runtime = await buildForkedModelRuntime(config, config, modelSelector); + return runWithForkedModelRuntime(runtime, fn); +} + // --------------------------------------------------------------------------- // ForkedQueryResult — returned by cache-path runForkedAgent // --------------------------------------------------------------------------- @@ -225,7 +314,7 @@ export interface AgentPathParams { taskPrompt: string; /** System prompt defining the agent's persona and constraints. */ systemPrompt: string; - /** Model override (defaults to config.getFastModel() ?? config.getModel()). */ + /** Model override (defaults to fast model selector, then current model). */ model?: string; /** Maximum number of agent turns (default: unlimited). */ maxTurns?: number; @@ -317,52 +406,60 @@ export async function runForkedAgent( if ('cacheSafeParams' in params) { const { config, userMessage, cacheSafeParams, jsonSchema, abortSignal } = params; - const model = params.model ?? cacheSafeParams.model; - const chat = createForkedChat(config, cacheSafeParams); - - const requestConfig: GenerateContentConfig = { ...NO_TOOLS }; - if (abortSignal) requestConfig.abortSignal = abortSignal; - if (jsonSchema) { - requestConfig.responseMimeType = 'application/json'; - requestConfig.responseJsonSchema = jsonSchema; - } - - const stream = await chat.sendMessageStream( - model, - { message: [{ text: userMessage }], config: requestConfig }, - 'forked_query', + const modelSelector = params.model ?? cacheSafeParams.model; + const modelRuntime = await buildForkedModelRuntime( + config, + config, + modelSelector, ); - let fullText = ''; - let usage: ForkedQueryResult['usage'] = { - inputTokens: 0, - outputTokens: 0, - cacheHitTokens: 0, - }; + return runWithForkedModelRuntime(modelRuntime, async (model) => { + const chat = createForkedChat(config, cacheSafeParams); - for await (const event of stream) { - if (event.type !== StreamEventType.CHUNK) continue; - const response = event.value; - const text = response.candidates?.[0]?.content?.parts - ?.filter((p) => !(p as Record)['thought']) - .map((p) => p.text ?? '') - .join(''); - if (text) fullText += text; - if (response.usageMetadata) - usage = extractQueryUsage(response.usageMetadata); - } + const requestConfig: GenerateContentConfig = { ...NO_TOOLS }; + if (abortSignal) requestConfig.abortSignal = abortSignal; + if (jsonSchema) { + requestConfig.responseMimeType = 'application/json'; + requestConfig.responseJsonSchema = jsonSchema; + } + + const stream = await chat.sendMessageStream( + model, + { message: [{ text: userMessage }], config: requestConfig }, + 'forked_query', + ); + + let fullText = ''; + let usage: ForkedQueryResult['usage'] = { + inputTokens: 0, + outputTokens: 0, + cacheHitTokens: 0, + }; - const trimmed = fullText.trim() || null; - let jsonResult: Record | undefined; - if (jsonSchema && trimmed) { - try { - jsonResult = JSON.parse(trimmed) as Record; - } catch { - // non-JSON response despite schema constraint — treat as text + for await (const event of stream) { + if (event.type !== StreamEventType.CHUNK) continue; + const response = event.value; + const text = response.candidates?.[0]?.content?.parts + ?.filter((p) => !(p as Record)['thought']) + .map((p) => p.text ?? '') + .join(''); + if (text) fullText += text; + if (response.usageMetadata) + usage = extractQueryUsage(response.usageMetadata); } - } - return { text: trimmed, jsonResult, usage }; + const trimmed = fullText.trim() || null; + let jsonResult: Record | undefined; + if (jsonSchema && trimmed) { + try { + jsonResult = JSON.parse(trimmed) as Record; + } catch { + // non-JSON response despite schema constraint — treat as text + } + } + + return { text: trimmed, jsonResult, usage }; + }); } // ── AgentHeadless path ──────────────────────────────────────────────────── @@ -396,9 +493,15 @@ export async function runForkedAgent( systemPrompt: params.systemPrompt, initialMessages: params.extraHistory, }; + const modelSelector = + params.model ?? params.config.getFastModel?.() ?? params.config.getModel(); + const modelRuntime = await buildForkedModelRuntime( + params.config, + yoloConfig, + modelSelector, + ); const modelConfig: ModelConfig = { - model: - params.model ?? params.config.getFastModel() ?? params.config.getModel(), + model: modelRuntime.model, }; const runConfig: RunConfig = { max_turns: params.maxTurns, @@ -416,11 +519,15 @@ export async function runForkedAgent( runConfig, toolConfig, emitter, + undefined, + modelRuntime.runtimeView, ); const context = new ContextState(); context.set('task_prompt', params.taskPrompt); - await headless.execute(context, params.abortSignal); + await runWithForkedModelRuntime(modelRuntime, async () => { + await headless.execute(context, params.abortSignal); + }); const terminateReason = headless.getTerminateMode(); const finalText = headless.getFinalText() || undefined; diff --git a/packages/core/src/utils/modelId.test.ts b/packages/core/src/utils/modelId.test.ts index 7d66ce9052b..ecacc4f76c0 100644 --- a/packages/core/src/utils/modelId.test.ts +++ b/packages/core/src/utils/modelId.test.ts @@ -96,3 +96,121 @@ describe('resolveModelId', () => { ); }); }); + +describe('resolveModelId with configured model context', () => { + it('resolves bare model IDs under the current auth type when available', () => { + expect( + resolveModelId('deepseek-v4-flash', { + currentAuthType: AuthType.USE_OPENAI, + getAvailableModels: (authTypes) => + authTypes?.includes(AuthType.USE_OPENAI) + ? [{ id: 'deepseek-v4-flash', authType: AuthType.USE_OPENAI }] + : [ + { + id: 'deepseek-v4-flash', + authType: AuthType.USE_ANTHROPIC, + }, + ], + }), + ).toEqual({ + authType: AuthType.USE_OPENAI, + modelId: 'deepseek-v4-flash', + }); + }); + + it('resolves bare model IDs to another configured auth type when current auth does not own them', () => { + expect( + resolveModelId('deepseek-v4-flash', { + currentAuthType: AuthType.USE_ANTHROPIC, + getAvailableModels: (authTypes) => + authTypes?.includes(AuthType.USE_ANTHROPIC) + ? [] + : [{ id: 'deepseek-v4-flash', authType: AuthType.USE_OPENAI }], + }), + ).toEqual({ + authType: AuthType.USE_OPENAI, + modelId: 'deepseek-v4-flash', + }); + }); + + it('falls back to current auth type for bare model IDs with no configured match', () => { + expect( + resolveModelId('unknown-model', { + currentAuthType: AuthType.USE_ANTHROPIC, + getAvailableModels: () => [], + }), + ).toEqual({ + authType: AuthType.USE_ANTHROPIC, + modelId: 'unknown-model', + }); + }); + + it('resolves fast through an authType-prefixed fast model', () => { + expect( + resolveModelId('fast', { + currentAuthType: AuthType.USE_ANTHROPIC, + fastModel: 'openai:deepseek-v4-flash', + }), + ).toEqual({ + authType: AuthType.USE_OPENAI, + modelId: 'deepseek-v4-flash', + }); + }); + + it('falls back to current auth type for bare fast models without configured model context', () => { + expect( + resolveModelId('fast', { + currentAuthType: AuthType.USE_ANTHROPIC, + fastModel: 'deepseek-v4-flash', + }), + ).toEqual({ + authType: AuthType.USE_ANTHROPIC, + modelId: 'deepseek-v4-flash', + }); + }); + + it('resolves bare fast models under the current auth type when available', () => { + expect( + resolveModelId('fast', { + currentAuthType: AuthType.USE_OPENAI, + fastModel: 'deepseek-v4-flash', + getAvailableModels: (authTypes) => + authTypes?.includes(AuthType.USE_OPENAI) + ? [{ id: 'deepseek-v4-flash', authType: AuthType.USE_OPENAI }] + : [ + { + id: 'deepseek-v4-flash', + authType: AuthType.USE_ANTHROPIC, + }, + ], + }), + ).toEqual({ + authType: AuthType.USE_OPENAI, + modelId: 'deepseek-v4-flash', + }); + }); + + it('resolves bare fast models to their configured auth type when current auth does not own them', () => { + expect( + resolveModelId('fast', { + currentAuthType: AuthType.USE_ANTHROPIC, + fastModel: 'deepseek-v4-flash', + getAvailableModels: (authTypes) => + authTypes?.includes(AuthType.USE_ANTHROPIC) + ? [] + : [{ id: 'deepseek-v4-flash', authType: AuthType.USE_OPENAI }], + }), + ).toEqual({ + authType: AuthType.USE_OPENAI, + modelId: 'deepseek-v4-flash', + }); + }); + + it('returns undefined for fast when no fast model is configured', () => { + expect( + resolveModelId('fast', { + currentAuthType: AuthType.USE_OPENAI, + }), + ).toBeUndefined(); + }); +}); diff --git a/packages/core/src/utils/modelId.ts b/packages/core/src/utils/modelId.ts index 0c182bc4ead..637e752a402 100644 --- a/packages/core/src/utils/modelId.ts +++ b/packages/core/src/utils/modelId.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import type { Config } from '../config/config.js'; import { AuthType } from '../core/contentGenerator.js'; export interface ResolvedModelId { @@ -15,6 +16,14 @@ export interface ModelIdResolutionContext { currentModel?: string; currentAuthType?: AuthType; fastModel?: string; + getAvailableModels?: ( + authTypes?: AuthType[], + ) => readonly ModelIdAvailableModel[]; +} + +export interface ModelIdAvailableModel { + id: string; + authType: AuthType; } type ModelIdSelector = @@ -38,7 +47,8 @@ const AUTH_TYPES = new Set(Object.values(AuthType)); * Supported forms: * - omitted / inherit -> use parent conversation model * - fast -> use the configured fastModel - * - modelId -> use parent authType with the provided modelId + * - modelId -> use current authType when available, otherwise the first + * configured authType that contains the model * - authType:modelId -> use explicit authType and modelId */ export function resolveModelId( @@ -48,6 +58,21 @@ export function resolveModelId( return resolveModelIdSelector(parseModelIdSelector(model), context); } +/** + * Build a {@link ModelIdResolutionContext} from a {@link Config}, wiring the + * standard adapter calls (current model, current auth type, configured fast + * model, configured models per auth type) used by every runtime caller. + */ +export function buildModelIdContext(config: Config): ModelIdResolutionContext { + return { + currentModel: config.getModel?.(), + currentAuthType: config.getContentGeneratorConfig?.()?.authType, + fastModel: config.getFastModel?.(), + getAvailableModels: (authTypes) => + config.getAllConfiguredModels?.(authTypes) ?? [], + }; +} + function parseModelIdSelector(model: string | undefined): ModelIdSelector { const trimmed = model?.trim(); if (!trimmed || trimmed === 'inherit') { @@ -84,13 +109,33 @@ function parseModelIdSelector(model: string | undefined): ModelIdSelector { }; } +function resolveAuthTypeForBareModel( + modelId: string, + context: ModelIdResolutionContext, +): AuthType | undefined { + if (context.currentAuthType && context.getAvailableModels) { + const currentModels = context.getAvailableModels([context.currentAuthType]); + if (currentModels.some((model) => model.id === modelId)) { + return context.currentAuthType; + } + } + + const configuredModel = context.getAvailableModels + ? context.getAvailableModels().find((model) => model.id === modelId) + : undefined; + return configuredModel?.authType ?? context.currentAuthType; +} + function resolveModelIdSelector( selector: ModelIdSelector, context: ModelIdResolutionContext, ): ResolvedModelId | undefined { if (selector.kind === 'model') { + const authType = + selector.authType ?? + resolveAuthTypeForBareModel(selector.modelId, context); return { - ...(selector.authType ? { authType: selector.authType } : {}), + ...(authType ? { authType } : {}), modelId: selector.modelId, }; } diff --git a/packages/core/src/utils/sideQuery.ts b/packages/core/src/utils/sideQuery.ts index 4fa7d3e8211..f5fb0f17c22 100644 --- a/packages/core/src/utils/sideQuery.ts +++ b/packages/core/src/utils/sideQuery.ts @@ -20,7 +20,7 @@ export interface SideQueryJsonOptions { abortSignal: AbortSignal; /** * Override the model used for this query. Defaults to - * `config.getFastModelForSideQuery?.() ?? config.getFastModel?.() ?? config.getModel() ?? DEFAULT_QWEN_MODEL` + * `config.getFastModel?.() ?? config.getModel() ?? DEFAULT_QWEN_MODEL` * — side queries run on the fast model when one is configured, including * fast models registered under a different authType than the main session. * Pass an explicit value to pin to the main model (e.g. long-form @@ -63,7 +63,7 @@ export interface SideQueryTextOptions { abortSignal: AbortSignal; /** * Override the model used for this query. Defaults to - * `config.getFastModelForSideQuery?.() ?? config.getFastModel?.() ?? config.getModel() ?? DEFAULT_QWEN_MODEL` + * `config.getFastModel?.() ?? config.getModel() ?? DEFAULT_QWEN_MODEL` * — side queries run on the fast model when one is configured, including * fast models registered under a different authType than the main session. * Pass an explicit value to pin to the main model (e.g. long-form @@ -105,7 +105,6 @@ function buildDefaultPromptId(purpose?: string): string { function resolveDefaultModel(config: Config, override?: string): string { return ( override ?? - config.getFastModelForSideQuery?.() ?? config.getFastModel?.() ?? config.getModel() ?? DEFAULT_QWEN_MODEL