From 8ba05c69271457cbcb116542ce412d66ecec14f6 Mon Sep 17 00:00:00 2001 From: "haozhe.yang" Date: Sat, 29 Aug 2026 20:46:40 +0800 Subject: [PATCH] fix(agent-core-v2): degrade broken secondary-model pool entries to startup warnings --- .agents/skills/agent-core-dev/config.md | 2 +- .changeset/secondary-model-pool-soft-fail.md | 5 + docs/en/configuration/config-files.md | 6 +- docs/zh/configuration/config-files.md | 6 +- .../src/agent/tools/agent/agentTool.ts | 5 +- .../swarm/tools/agent-swarm/agentSwarmTool.ts | 5 +- .../features/tower/tools/spawn/spawnTool.ts | 1 + packages/agent-core-v2/src/index.ts | 2 - .../src/session/subagent/configSection.ts | 150 +++--- .../subagent/subagentModelsValidation.ts | 10 - .../subagentModelsValidationService.ts | 30 -- .../src/session/subagent/subagentService.ts | 1 + .../sessionLifecycleService.ts | 13 +- .../test/app/config/config.test.ts | 61 +-- .../test/features/swarm/swarm.test.ts | 71 +-- .../test/session/subagent/spawn.test.ts | 20 +- .../subagent/subagentModelsValidation.test.ts | 434 +++++++++++------- packages/agent-core-v2/test/tool/tool.test.ts | 11 +- packages/kap-server/src/routes/sessions.ts | 18 +- packages/kap-server/test/config.test.ts | 27 +- packages/node-sdk/src/sdk-rpc-client-v2.ts | 28 +- 21 files changed, 543 insertions(+), 363 deletions(-) create mode 100644 .changeset/secondary-model-pool-soft-fail.md delete mode 100644 packages/agent-core-v2/src/session/subagent/subagentModelsValidation.ts delete mode 100644 packages/agent-core-v2/src/session/subagent/subagentModelsValidationService.ts diff --git a/.agents/skills/agent-core-dev/config.md b/.agents/skills/agent-core-dev/config.md index c84111259f1..757e79c0ba6 100644 --- a/.agents/skills/agent-core-dev/config.md +++ b/.agents/skills/agent-core-dev/config.md @@ -101,7 +101,7 @@ pass `ConfigTarget.Memory` for a per-run override that is never written to disk. - `src/kosong/model/thinking.ts` (owner domain, not `config`) — the `resolveThinkingEffort` helper and the authoritative `ThinkingConfig` type (the `thinking` section itself registers from `src/app/kosongConfig/configSection.ts`). - `src/app/config/configPure.ts` — `isPlainObject`, `deepMerge`, `omitUndefined`, `describeUnknownError`. -A domain that owns a section keeps the schema in its own `configSection.ts` (e.g. `src/app/flag/flag.ts` for `experimental`, `src/agent/loop/configSection.ts` for `loopControl`). Exception: kosong-owned sections (`providers`, `models`, `thinking`) — kosong is a pure, persistence-free abstraction layer that defines only the types (`src/kosong/{provider,model}`); the section constants, the zod schemas (re-derived from those types and compile-time pinned via `AssertExact, Type>>`, see `_base/utils/typeEquality.ts`), the registrations, env bindings, and TOML transforms all live in the persistence wrapper `src/app/kosongConfig/configSection.ts`. (`modelCatalog` has no kosong-side type at all — its section is fully self-contained in `app/kosongConfig`, types derived from the schema.) A cross-section env overlay (e.g. the `KIMI_MODEL_*` synthesis in `src/app/kosongConfig/envOverlay.ts`) lives in the wrapper too and is registered via module-level `registerConfigOverlay`. The session subagent domain owns two sections in `src/session/subagent/configSection.ts`: `[subagent]` (`timeout_ms` on disk) and `[secondary_model]` (`default_model` plus the `[secondary_model.models]` pool, with a lone legacy v1 `model` key honored as a fallback default below `default_model`); neither carries a cross-section overlay. Cross-field pool validation (default present / in-pool / every key resolvable) runs at session creation in `subagentModelsValidationService.ts`, not in the schema. The two-way sync between config sections and kosong's in-memory registries is owned by `IKosongConfigService` (`src/app/kosongConfig/kosongConfigService.ts`). +A domain that owns a section keeps the schema in its own `configSection.ts` (e.g. `src/app/flag/flag.ts` for `experimental`, `src/agent/loop/configSection.ts` for `loopControl`). Exception: kosong-owned sections (`providers`, `models`, `thinking`) — kosong is a pure, persistence-free abstraction layer that defines only the types (`src/kosong/{provider,model}`); the section constants, the zod schemas (re-derived from those types and compile-time pinned via `AssertExact, Type>>`, see `_base/utils/typeEquality.ts`), the registrations, env bindings, and TOML transforms all live in the persistence wrapper `src/app/kosongConfig/configSection.ts`. (`modelCatalog` has no kosong-side type at all — its section is fully self-contained in `app/kosongConfig`, types derived from the schema.) A cross-section env overlay (e.g. the `KIMI_MODEL_*` synthesis in `src/app/kosongConfig/envOverlay.ts`) lives in the wrapper too and is registered via module-level `registerConfigOverlay`. The session subagent domain owns two sections in `src/session/subagent/configSection.ts`: `[subagent]` (`timeout_ms` on disk) and `[secondary_model]` (`default_model` plus the `[secondary_model.models]` pool, with a lone legacy v1 `model` key honored as a fallback default below `default_model`); neither carries a cross-section overlay. Pool problems (default missing / not in-pool / unresolvable key) never block session creation: `resolveEffectiveSubagentModelPool` in the same `configSection.ts` filters broken entries, falls back to the first surviving entry, and reports issues that surface as startup warnings; a spawn that still binds a broken model fails lazily at spawn time. The two-way sync between config sections and kosong's in-memory registries is owned by `IKosongConfigService` (`src/app/kosongConfig/kosongConfigService.ts`). ## Scope diff --git a/.changeset/secondary-model-pool-soft-fail.md b/.changeset/secondary-model-pool-soft-fail.md new file mode 100644 index 00000000000..e7b2f238baa --- /dev/null +++ b/.changeset/secondary-model-pool-soft-fail.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix sessions failing to open when the subagent model pool contains an entry that no longer resolves to a configured model. diff --git a/docs/en/configuration/config-files.md b/docs/en/configuration/config-files.md index 71d802dd69c..f4bf0fdfbb8 100644 --- a/docs/en/configuration/config-files.md +++ b/docs/en/configuration/config-files.md @@ -220,7 +220,7 @@ Constraints between the fields: - `default_effort` is section-wide: every spawn binds it regardless of the chosen pool entry (or the forced model). For per-entry efforts, leave it unset and use model variants (see below). - `primary` is a reserved alias (see below) and cannot be a pool key. -Pool aliases reference the current `[models]` table: if a provider is later deleted or logged out, or its refreshed model list no longer contains an alias, session startup fails with a configuration error naming the broken alias — fix or remove the entry to recover. The `[secondary_model]` section itself is never rewritten automatically. +Pool aliases reference the current `[models]` table: if a provider is later deleted or logged out, or its refreshed model list no longer contains an alias, the broken entry is skipped instead of blocking startup — the session opens with a warning naming the alias, and the pool carries on with the remaining entries. When `default_model` itself breaks, the first remaining entry becomes the effective default; when no entry survives, subagents inherit the caller's model. Only a spawn that still ends up bound to a broken model — an explicit `model` request naming it, or a `force` pin — fails with an error naming the entry. The `[secondary_model]` section itself is never rewritten automatically. In the interactive TUI, the [`/secondary-model`](../reference/slash-commands.md) command (alias `/subagent-model`) opens a model selector: the choice is written to `default_model` (when a models table exists and the picked alias is not in it, an entry with an empty description is added), and newly spawned subagents pick up the new default immediately — no session restart needed. @@ -293,10 +293,12 @@ Two prerequisites: Note the asymmetry between the main agent and pool-bound subagents: for the main agent, a configured global `[thinking].effort` overrides the variant's `default_effort`; for subagents the variant's `default_effort` wins over the global value, and only `[secondary_model].default_effort` outranks it. Value and fallback rules follow the [`[models]` entry's `default_effort`](#models). ::: warning Note -Configuration errors fail loudly instead of falling back silently. Session creation, resume, and fork all fail at startup when: +Pool problems never block session startup. Session creation, resume, and fork succeed with a startup warning when: - `default_model` is missing, is not a pool key, or a pool key does not resolve to a configured [`[models]`](#models) entry; - `force` is set without `default_model`, or combined with a `models` table. + +Unresolvable entries are skipped: they disappear from the model choices advertised to the main agent, and a broken `default_model` falls back to the first remaining entry. Only a spawn that still binds a broken model — an explicit `model` request naming it, or a `force` pin — fails with an error naming the entry. ::: ## `thinking` diff --git a/docs/zh/configuration/config-files.md b/docs/zh/configuration/config-files.md index 597b352cdf5..6f162ac4fb2 100644 --- a/docs/zh/configuration/config-files.md +++ b/docs/zh/configuration/config-files.md @@ -220,7 +220,7 @@ default_model = "kimi-code/kimi-for-coding-highspeed" - `default_effort` 是节级设置：无论派生绑定到池中哪个条目（或 force 固定的模型）都生效。想按条目区分档位时不要设置它，改用下文的模型「变体」。 - `primary` 是保留字（含义见下文），不能作为池中 key。 -池别名引用的是 `[models]` 表的当前内容：如果之后删除供应商、登出账号，或其刷新后的模型列表不再包含某个别名，会话启动时会报出指明失效别名的配置错误，修正或移除对应条目即可恢复。系统不会自动改写 `[secondary_model]` 节。 +池别名引用的是 `[models]` 表的当前内容：如果之后删除供应商、登出账号，或其刷新后的模型列表不再包含某个别名，失效条目会被跳过而不是阻塞启动——会话照常打开并给出指明该别名的警告，池继续以剩余条目工作。当 `default_model` 本身失效时，剩余的第一个条目成为实际默认值；当所有条目都失效时，subagent 继承调用方的模型。只有仍然绑定到失效模型的派生——显式以 `model` 参数指定它，或 `force` 固定——才会报错并指明该条目。系统不会自动改写 `[secondary_model]` 节。 在交互式 TUI 中，也可以用 [`/secondary-model`](../reference/slash-commands.md) 命令（别名 `/subagent-model`）打开模型选择器：选择后写入 `default_model`（已有 models 表而所选别名不在其中时，会一并补一条空描述条目），之后派生的 subagent 立即按新默认值绑定，无需重启会话。 @@ -292,10 +292,12 @@ k3-max = "同一模型的 max Thinking 档位。适合最难的子任务。" 另外注意 main agent 与 subagent 的不对称：对 main agent，全局 `[thinking].effort` 一旦设置就压过变体的 `default_effort`；对绑定池内别名的 subagent，变体的 `default_effort` 优先于全局值，只有 `[secondary_model].default_effort` 的优先级更高。取值与回落规则同 [`[models]` 条目的 `default_effort`](#models)。 ::: warning 注意 -配置错误一律直接报错，不做静默回退。出现以下情况时，会话的创建、恢复（resume）与 fork 都会在启动时失败： +池配置问题不会阻塞会话启动。出现以下情况时，会话的创建、恢复（resume）与 fork 都会正常进行，并给出启动警告： - `default_model` 缺失、不是池中 key，或池中 key 无法解析到已配置的 [`[models]`](#models) 条目； - `force` 未搭配 `default_model`，或与 `models` 表同时使用。 + +无法解析的条目会被跳过：它们不再出现在提供给 main agent 的模型选项中，`default_model` 失效时回落到剩余的第一个条目。只有仍然绑定到失效模型的派生——显式以 `model` 参数指定它，或 `force` 固定——才会报错并指明该条目。 ::: ## `thinking` diff --git a/packages/agent-core-v2/src/agent/tools/agent/agentTool.ts b/packages/agent-core-v2/src/agent/tools/agent/agentTool.ts index b68ad9fb712..d5d2df78924 100644 --- a/packages/agent-core-v2/src/agent/tools/agent/agentTool.ts +++ b/packages/agent-core-v2/src/agent/tools/agent/agentTool.ts @@ -41,6 +41,7 @@ import { import { ILogService } from '#/_base/log/log'; import { IConfigService } from '#/app/config/config'; import { IFlagService } from '#/app/flag/flag'; +import { IModelCatalog } from '#/kosong/model/catalog'; import { IAgentLifecycleService } from '#/session/agentLifecycle/agentLifecycle'; import { isSubagentMeta, subagentLabels, subagentParentAgentId } from '#/session/agentLifecycle/subagentMetadata'; import { ISessionMetadata } from '#/session/sessionMetadata/sessionMetadata'; @@ -84,7 +85,7 @@ export class SubagentTool implements ISubagentTool { readonly name: string = 'Agent'; get parameters(): Record { - const parameters = exposesSubagentModelChoice(this.config, this.flags) + const parameters = exposesSubagentModelChoice(this.config, this.flags, this.modelCatalog) ? SUBAGENT_TOOL_PARAMETERS : SUBAGENT_TOOL_PARAMETERS_NO_MODEL; return this.flags.enabled(SUBAGENT_FORK_FLAG_ID) @@ -110,6 +111,7 @@ export class SubagentTool implements ISubagentTool { @ILogService private readonly log: ILogService, @IConfigService private readonly config: IConfigService, @IFlagService private readonly flags: IFlagService, + @IModelCatalog private readonly modelCatalog: IModelCatalog, @AgentToolContribution private readonly contributions: CollectionView, ) { this.callerAgentId = scopeContext.agentId; @@ -150,6 +152,7 @@ export class SubagentTool implements ISubagentTool { this.config, this.flags, this.profile.data().modelAlias, + this.modelCatalog, ); if (modelLines !== undefined) { description += `\n\n${modelLines}`; diff --git a/packages/agent-core-v2/src/features/swarm/tools/agent-swarm/agentSwarmTool.ts b/packages/agent-core-v2/src/features/swarm/tools/agent-swarm/agentSwarmTool.ts index 997fd27549e..740f95bd5f2 100644 --- a/packages/agent-core-v2/src/features/swarm/tools/agent-swarm/agentSwarmTool.ts +++ b/packages/agent-core-v2/src/features/swarm/tools/agent-swarm/agentSwarmTool.ts @@ -8,6 +8,7 @@ import { Error2, ErrorCodes } from '#/errors'; import { toInputJsonSchema } from '#/tool/input-schema'; import { IConfigService } from '#/app/config/config'; import { IFlagService } from '#/app/flag/flag'; +import { IModelCatalog } from '#/kosong/model/catalog'; import { ISessionSwarmService, type SessionSwarmTask } from '#/features/swarm/session/sessionSwarm'; import { IAgentProfileService } from '#/agent/profile/profile'; import { IAgentScopeContext } from '#/agent/scopeContext/scopeContext'; @@ -73,7 +74,7 @@ export class AgentSwarmTool implements IAgentSwarmTool { readonly name = 'AgentSwarm' as const; get parameters(): Record { - const parameters = exposesSubagentModelChoice(this.config, this.flags) + const parameters = exposesSubagentModelChoice(this.config, this.flags, this.modelCatalog) ? AGENT_SWARM_PARAMETERS : AGENT_SWARM_PARAMETERS_NO_MODEL; return this.flags.enabled(SUBAGENT_FORK_FLAG_ID) @@ -89,6 +90,7 @@ export class AgentSwarmTool implements IAgentSwarmTool { @IAgentSwarmService private readonly swarmMode: IAgentSwarmService, @IConfigService private readonly config: IConfigService, @IFlagService private readonly flags: IFlagService, + @IModelCatalog private readonly modelCatalog: IModelCatalog, @ISessionSubagentService private readonly subagents: ISessionSubagentService, @IAgentProfileService private readonly profile: IAgentProfileService, ) { @@ -104,6 +106,7 @@ export class AgentSwarmTool implements IAgentSwarmTool { this.config, this.flags, this.profile.data().modelAlias, + this.modelCatalog, ); return modelLines === undefined ? description : `${description}\n\n${modelLines}`; } diff --git a/packages/agent-core-v2/src/features/tower/tools/spawn/spawnTool.ts b/packages/agent-core-v2/src/features/tower/tools/spawn/spawnTool.ts index a50eb5c83be..02a9d04c4ff 100644 --- a/packages/agent-core-v2/src/features/tower/tools/spawn/spawnTool.ts +++ b/packages/agent-core-v2/src/features/tower/tools/spawn/spawnTool.ts @@ -167,6 +167,7 @@ export class TowerSpawnTool implements ITowerSpawnTool { this.flags, { modelAlias: own.modelAlias, thinkingLevel: own.thinkingLevel }, args.kind === 'reviewer' ? 'primary' : undefined, + this.modelCatalog, ); let handle: SubagentHandle; try { diff --git a/packages/agent-core-v2/src/index.ts b/packages/agent-core-v2/src/index.ts index d5a646d23cd..dce4984f352 100644 --- a/packages/agent-core-v2/src/index.ts +++ b/packages/agent-core-v2/src/index.ts @@ -478,8 +478,6 @@ export * from '#/session/subagent/subagent'; export * from '#/session/subagent/subagentService'; export * from '#/session/subagent/spawn'; import '#/session/subagent/flag'; -export * from '#/session/subagent/subagentModelsValidation'; -import '#/session/subagent/subagentModelsValidationService'; export * from '#/agent/tools/agent/subagent-task'; export { AGENT_RUN_PROMPT_ORIGIN } from '#/session/subagent/runAgentTurn'; export * from '#/session/subagent/mirrorAgentRun'; diff --git a/packages/agent-core-v2/src/session/subagent/configSection.ts b/packages/agent-core-v2/src/session/subagent/configSection.ts index 9e27bde6c06..d58beb26020 100644 --- a/packages/agent-core-v2/src/session/subagent/configSection.ts +++ b/packages/agent-core-v2/src/session/subagent/configSection.ts @@ -111,84 +111,106 @@ export function isSubagentModelForced(config: IConfigService): boolean { return config.get(SECONDARY_MODEL_SECTION)?.force === true; } -export function exposesSubagentModelChoice(config: IConfigService, flags: IFlagService): boolean { - if (!flags.enabled(SECONDARY_MODEL_FLAG_ID)) return false; - if (isSubagentModelForced(config)) return false; - return resolveSubagentModelPool(config) !== undefined; -} - export const SECONDARY_MODEL_DEFAULT_MODEL_REQUIRED_MESSAGE = '[secondary_model].default_model is required when [secondary_model.models] is configured'; export const SECONDARY_MODEL_PRIMARY_MODEL_RESERVED_MESSAGE = `[secondary_model.models] key "${PRIMARY_SUBAGENT_MODEL_CHOICE}" is reserved: it always binds the caller's own model. Rename the pool entry.`; -export function assertValidSubagentModelPool( - pool: SubagentModelPool, +export interface SubagentModelPoolResolution { + readonly pool: SubagentModelPool | undefined; + readonly issues: readonly string[]; +} + +export function resolveEffectiveSubagentModelPool( + config: IConfigService, + flags: IFlagService, modelCatalog: IModelCatalog, -): void { - if (Object.hasOwn(pool.models, PRIMARY_SUBAGENT_MODEL_CHOICE)) { - throw new Error2(ErrorCodes.CONFIG_INVALID, SECONDARY_MODEL_PRIMARY_MODEL_RESERVED_MESSAGE, { - details: { - section: SECONDARY_MODEL_SECTION, - field: 'models', - model: PRIMARY_SUBAGENT_MODEL_CHOICE, - }, - }); - } - const aliases = Object.keys(pool.models); - if (pool.defaultModel === undefined) { - throw new Error2(ErrorCodes.CONFIG_INVALID, SECONDARY_MODEL_DEFAULT_MODEL_REQUIRED_MESSAGE, { - details: { section: SECONDARY_MODEL_SECTION, field: 'defaultModel' }, - }); - } - if (!Object.hasOwn(pool.models, pool.defaultModel)) { - throw new Error2( - ErrorCodes.CONFIG_INVALID, - `[secondary_model].default_model "${pool.defaultModel}" is not a [secondary_model.models] key. Available models: ${aliases.join(', ')}.`, - { details: { model: pool.defaultModel, availableModels: aliases } }, - ); +): SubagentModelPoolResolution { + if (!flags.enabled(SECONDARY_MODEL_FLAG_ID)) return { pool: undefined, issues: [] }; + const section = config.get(SECONDARY_MODEL_SECTION); + if (section?.force === true) { + const issues: string[] = []; + if (section.models !== undefined) { + issues.push(SECONDARY_MODEL_FORCE_EXCLUDES_MODELS_MESSAGE); + } + const forcedModel = section.defaultModel ?? section.model; + if (forcedModel === undefined) { + issues.push(SECONDARY_MODEL_FORCE_REQUIRES_DEFAULT_MESSAGE); + } else { + try { + modelCatalog.get(forcedModel); + } catch (error) { + issues.push( + `[secondary_model] forced model "${forcedModel}" could not be resolved: ${error instanceof Error ? error.message : String(error)}. Subagent spawns will fail until this is fixed.`, + ); + } + } + return { pool: undefined, issues }; } - for (const alias of aliases) { + const raw = resolveSubagentModelPool(config); + if (raw === undefined) return { pool: undefined, issues: [] }; + const issues: string[] = []; + const models: Record = {}; + for (const [alias, description] of Object.entries(raw.models)) { + if (alias === PRIMARY_SUBAGENT_MODEL_CHOICE) { + issues.push( + `${SECONDARY_MODEL_PRIMARY_MODEL_RESERVED_MESSAGE} The entry is ignored until renamed.`, + ); + continue; + } try { modelCatalog.get(alias); + models[alias] = description; } catch (error) { - throw new Error2( - ErrorCodes.CONFIG_INVALID, - `[secondary_model.models] entry "${alias}" could not be resolved: ${error instanceof Error ? error.message : String(error)}`, - { cause: error, details: { model: alias } }, + issues.push( + `[secondary_model.models] entry "${alias}" could not be resolved: ${error instanceof Error ? error.message : String(error)}. The entry is ignored until fixed.`, ); } } + const aliases = Object.keys(models); + if (aliases.length === 0) { + if (raw.defaultModel === undefined) { + issues.push( + `${SECONDARY_MODEL_DEFAULT_MODEL_REQUIRED_MESSAGE}; subagents inherit the caller's model until fixed.`, + ); + } else if (!Object.hasOwn(raw.models, raw.defaultModel)) { + issues.push( + `[secondary_model].default_model "${raw.defaultModel}" is not a [secondary_model.models] key; subagents inherit the caller's model until fixed.`, + ); + } + return { pool: undefined, issues }; + } + let defaultModel = + raw.defaultModel !== undefined && Object.hasOwn(models, raw.defaultModel) + ? raw.defaultModel + : undefined; + if (defaultModel === undefined) { + defaultModel = aliases[0]!; + issues.push( + raw.defaultModel === undefined + ? `${SECONDARY_MODEL_DEFAULT_MODEL_REQUIRED_MESSAGE}; falling back to "${defaultModel}" until fixed.` + : `[secondary_model].default_model "${raw.defaultModel}" is not available; falling back to "${defaultModel}" until fixed.`, + ); + } + return { pool: { defaultModel, models }, issues }; } -export function assertValidSubagentModelConfig( +export function exposesSubagentModelChoice( config: IConfigService, flags: IFlagService, modelCatalog: IModelCatalog, -): void { - if (!flags.enabled(SECONDARY_MODEL_FLAG_ID)) return; - const section = config.get(SECONDARY_MODEL_SECTION); - if (section?.force === true) { - if (section.models !== undefined) { - throw new Error2(ErrorCodes.CONFIG_INVALID, SECONDARY_MODEL_FORCE_EXCLUDES_MODELS_MESSAGE, { - details: { section: SECONDARY_MODEL_SECTION, field: 'force' }, - }); - } - if (section.defaultModel === undefined && section.model === undefined) { - throw new Error2(ErrorCodes.CONFIG_INVALID, SECONDARY_MODEL_FORCE_REQUIRES_DEFAULT_MESSAGE, { - details: { section: SECONDARY_MODEL_SECTION, field: 'defaultModel' }, - }); - } - } - const pool = resolveSubagentModelPool(config); - if (pool !== undefined) assertValidSubagentModelPool(pool, modelCatalog); +): boolean { + if (!flags.enabled(SECONDARY_MODEL_FLAG_ID)) return false; + if (isSubagentModelForced(config)) return false; + return resolveEffectiveSubagentModelPool(config, flags, modelCatalog).pool !== undefined; } export function resolveSubagentBinding( config: IConfigService, flags: IFlagService, own: { modelAlias: string; thinkingLevel: string }, - requested?: string, + requested: string | undefined, + modelCatalog: IModelCatalog, ): { model: string; thinking?: string } { const enabled = flags.enabled(SECONDARY_MODEL_FLAG_ID); const section = config.get(SECONDARY_MODEL_SECTION); @@ -216,7 +238,9 @@ export function resolveSubagentBinding( if (requested === PRIMARY_SUBAGENT_MODEL_CHOICE) { return { model: own.modelAlias, thinking: own.thinkingLevel }; } - const pool = enabled ? resolveSubagentModelPool(config) : undefined; + const pool = enabled + ? resolveEffectiveSubagentModelPool(config, flags, modelCatalog).pool + : undefined; if (pool === undefined) { if (requested !== undefined) { throw new Error2( @@ -227,15 +251,6 @@ export function resolveSubagentBinding( } return { model: own.modelAlias, thinking: own.thinkingLevel }; } - if (Object.hasOwn(pool.models, PRIMARY_SUBAGENT_MODEL_CHOICE)) { - throw new Error2(ErrorCodes.CONFIG_INVALID, SECONDARY_MODEL_PRIMARY_MODEL_RESERVED_MESSAGE, { - details: { - section: SECONDARY_MODEL_SECTION, - field: 'models', - model: PRIMARY_SUBAGENT_MODEL_CHOICE, - }, - }); - } const choice = requested ?? pool.defaultModel; if (choice === undefined) { throw new Error2(ErrorCodes.CONFIG_INVALID, SECONDARY_MODEL_DEFAULT_MODEL_REQUIRED_MESSAGE, { @@ -267,9 +282,12 @@ export function buildSubagentModelDescriptions( config: IConfigService, flags: IFlagService, callerModelAlias: string | undefined, + modelCatalog: IModelCatalog, ): string | undefined { - if (!exposesSubagentModelChoice(config, flags)) return undefined; - const pool = resolveSubagentModelPool(config)!; + if (!flags.enabled(SECONDARY_MODEL_FLAG_ID)) return undefined; + if (isSubagentModelForced(config)) return undefined; + const pool = resolveEffectiveSubagentModelPool(config, flags, modelCatalog).pool; + if (pool === undefined) return undefined; const lines = ['Available models (pass via model):']; const defaultModel = pool.defaultModel; const markersFor = (alias: string): string => { diff --git a/packages/agent-core-v2/src/session/subagent/subagentModelsValidation.ts b/packages/agent-core-v2/src/session/subagent/subagentModelsValidation.ts deleted file mode 100644 index e49035d6c9a..00000000000 --- a/packages/agent-core-v2/src/session/subagent/subagentModelsValidation.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation'; - -export interface ISessionSubagentModelsValidationService { - readonly _serviceBrand: undefined; -} - -export const ISessionSubagentModelsValidationService: ServiceIdentifier = - createDecorator( - 'sessionSubagentModelsValidationService', - ); diff --git a/packages/agent-core-v2/src/session/subagent/subagentModelsValidationService.ts b/packages/agent-core-v2/src/session/subagent/subagentModelsValidationService.ts deleted file mode 100644 index cd19ad03951..00000000000 --- a/packages/agent-core-v2/src/session/subagent/subagentModelsValidationService.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { LifecycleScope } from '#/app/scopes'; -import { ScopeActivation, registerScopedService } from '#/_base/di/scope'; -import { IConfigService } from '#/app/config/config'; -import { IFlagService } from '#/app/flag/flag'; -import { IModelCatalog } from '#/kosong/model/catalog'; - -import { assertValidSubagentModelConfig } from './configSection'; -import { ISessionSubagentModelsValidationService } from './subagentModelsValidation'; - -export class SessionSubagentModelsValidationService - implements ISessionSubagentModelsValidationService -{ - declare readonly _serviceBrand: undefined; - - constructor( - @IConfigService config: IConfigService, - @IFlagService flags: IFlagService, - @IModelCatalog modelCatalog: IModelCatalog, - ) { - assertValidSubagentModelConfig(config, flags, modelCatalog); - } -} - -registerScopedService( - LifecycleScope.Session, - ISessionSubagentModelsValidationService, - SessionSubagentModelsValidationService, - ScopeActivation.OnScopeCreated, - 'subagent', -); diff --git a/packages/agent-core-v2/src/session/subagent/subagentService.ts b/packages/agent-core-v2/src/session/subagent/subagentService.ts index f97b6c71866..b39d0e08339 100644 --- a/packages/agent-core-v2/src/session/subagent/subagentService.ts +++ b/packages/agent-core-v2/src/session/subagent/subagentService.ts @@ -136,6 +136,7 @@ export class SessionSubagentService extends Service implements ISessionSubagentS this.flags, { modelAlias: own.modelAlias, thinkingLevel: own.thinkingLevel }, input.model, + this.modelCatalog, ); let model: Model; try { diff --git a/packages/agent-core-v2/src/workspace/sessionLifecycle/sessionLifecycleService.ts b/packages/agent-core-v2/src/workspace/sessionLifecycle/sessionLifecycleService.ts index a6b2ac436e3..c1bf49ef0ea 100644 --- a/packages/agent-core-v2/src/workspace/sessionLifecycle/sessionLifecycleService.ts +++ b/packages/agent-core-v2/src/workspace/sessionLifecycle/sessionLifecycleService.ts @@ -61,7 +61,7 @@ import { IModelCatalog } from '#/kosong/model/catalog'; import { IModelService } from '#/kosong/model/model'; import { IProviderService } from '#/kosong/provider/provider'; import { IFlagService } from '#/app/flag/flag'; -import { assertValidSubagentModelConfig } from '#/session/subagent/configSection'; +import { resolveEffectiveSubagentModelPool } from '#/session/subagent/configSection'; import { IWorkspaceContext } from '#/workspace/workspaceContext/workspaceContext'; import { IUserAgentProfileLoader } from '#/workspace/workspaceAgentProfileLoader/userAgentProfileLoader'; import { IPluginAgentProfileLoader } from '#/workspace/workspaceAgentProfileLoader/pluginAgentProfileLoader'; @@ -224,9 +224,12 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec return handle; } - private async assertSubagentModelPoolPreFlight(): Promise { + private async logSubagentModelPoolIssues(): Promise { await Promise.all([this.config.ready, this.models.ready, this.providers.ready]); - assertValidSubagentModelConfig(this.config, this.flags, this.modelCatalog); + const { issues } = resolveEffectiveSubagentModelPool(this.config, this.flags, this.modelCatalog); + for (const issue of issues) { + this.log.warn(issue); + } } private async materializeSession(opts: MaterializeSessionOptions): Promise { @@ -234,7 +237,7 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec const sessionScope = sessionScopeOf(this.handlerScope, opts.sessionId); const sessionDir = sessionDirOf(this.bootstrap.homeDir, this.handlerScope, opts.sessionId); const metaScope = sessionScope; - await this.assertSubagentModelPoolPreFlight(); + await this.logSubagentModelPoolIssues(); await this.workspaceDirs.ready; await this.workspaceDirs.mergeAdditionalDirs(opts.workDir, opts.additionalDirs ?? []); const ctx: ISessionContext = { @@ -493,7 +496,7 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec let target: ISessionScopeHandle | undefined; let targetSessionDir: string | undefined; try { - await this.assertSubagentModelPoolPreFlight(); + await this.logSubagentModelPoolIssues(); await drainSessionMetadataWrites(); const sourceMeta = sourceHandle !== undefined diff --git a/packages/agent-core-v2/test/app/config/config.test.ts b/packages/agent-core-v2/test/app/config/config.test.ts index 5b1dd8bda39..0a90505ce80 100644 --- a/packages/agent-core-v2/test/app/config/config.test.ts +++ b/packages/agent-core-v2/test/app/config/config.test.ts @@ -94,6 +94,7 @@ import { wrapSubagentModelError, } from '#/session/subagent/configSection'; import { SECONDARY_MODEL_FLAG_ID } from '#/session/subagent/flag'; +import { IModelCatalog, type Model } from '#/kosong/model/catalog'; import { DEFAULT_SWARM_TIMEOUT_MS, resolveSwarmTimeoutMs, @@ -130,6 +131,11 @@ function secondaryModelFlags(enabled = true) { return stubFlag((id) => enabled && id === SECONDARY_MODEL_FLAG_ID); } +const permissiveModelCatalog = { + _serviceBrand: undefined, + get: (id: string) => ({ id }) as Model, +} as unknown as IModelCatalog; + const TEST_OS_ENV = { osKind: 'Linux', osArch: 'x86_64', @@ -1871,11 +1877,11 @@ describe('subagent config section', () => { const own = { modelAlias: 'provider/main', thinkingLevel: 'medium' }; const noPool = await createConfig({}); - expect(resolveSubagentBinding(noPool.config, secondaryModelFlags(), own)).toEqual({ + expect(resolveSubagentBinding(noPool.config, secondaryModelFlags(), own, undefined, permissiveModelCatalog)).toEqual({ model: 'provider/main', thinking: 'medium', }); - expect(resolveSubagentBinding(noPool.config, secondaryModelFlags(), own, 'primary')).toEqual({ + expect(resolveSubagentBinding(noPool.config, secondaryModelFlags(), own, 'primary', permissiveModelCatalog)).toEqual({ model: 'provider/main', thinking: 'medium', }); @@ -1885,15 +1891,15 @@ describe('subagent config section', () => { {}, '[secondary_model]\ndefault_model = "provider/fast"\n\n[secondary_model.models]\n"provider/fast" = "fast and cheap"\n"provider/smart" = "hard tasks"\n', ); - expect(resolveSubagentBinding(pool.config, secondaryModelFlags(), own)).toEqual({ + expect(resolveSubagentBinding(pool.config, secondaryModelFlags(), own, undefined, permissiveModelCatalog)).toEqual({ model: 'provider/fast', thinking: undefined, }); - expect(resolveSubagentBinding(pool.config, secondaryModelFlags(), own, 'provider/smart')).toEqual({ + expect(resolveSubagentBinding(pool.config, secondaryModelFlags(), own, 'provider/smart', permissiveModelCatalog)).toEqual({ model: 'provider/smart', thinking: undefined, }); - expect(resolveSubagentBinding(pool.config, secondaryModelFlags(), own, 'primary')).toEqual({ + expect(resolveSubagentBinding(pool.config, secondaryModelFlags(), own, 'primary', permissiveModelCatalog)).toEqual({ model: 'provider/main', thinking: 'medium', }); @@ -1907,12 +1913,12 @@ describe('subagent config section', () => { '[secondary_model]\ndefault_model = "provider/fast"\nforce = true\n\n[secondary_model.models]\n"provider/fast" = "fast and cheap"\n', ); - expect(resolveSubagentBinding(config, secondaryModelFlags(false), own)).toEqual({ + expect(resolveSubagentBinding(config, secondaryModelFlags(false), own, undefined, permissiveModelCatalog)).toEqual({ model: 'provider/main', thinking: 'medium', }); expect(() => - resolveSubagentBinding(config, secondaryModelFlags(false), own, 'provider/fast'), + resolveSubagentBinding(config, secondaryModelFlags(false), own, 'provider/fast', permissiveModelCatalog), ).toThrow(/no \[secondary_model\.models\] pool is configured/); disposables.dispose(); @@ -1925,15 +1931,15 @@ describe('subagent config section', () => { '[secondary_model]\ndefault_model = "provider/fast"\n', ); - expect(resolveSubagentBinding(config, secondaryModelFlags(), own)).toEqual({ + expect(resolveSubagentBinding(config, secondaryModelFlags(), own, undefined, permissiveModelCatalog)).toEqual({ model: 'provider/fast', thinking: undefined, }); - expect(resolveSubagentBinding(config, secondaryModelFlags(), own, 'primary')).toEqual({ + expect(resolveSubagentBinding(config, secondaryModelFlags(), own, 'primary', permissiveModelCatalog)).toEqual({ model: 'provider/main', thinking: 'medium', }); - expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own, 'provider/smart')).toThrow( + expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own, 'provider/smart', permissiveModelCatalog)).toThrow( /Invalid model "provider\/smart"\. Available models: provider\/fast, primary\./, ); @@ -1955,11 +1961,11 @@ describe('subagent config section', () => { defaultModel: 'provider/fast', models: { 'provider/fast': '' }, }); - expect(resolveSubagentBinding(config, secondaryModelFlags(), own)).toEqual({ + expect(resolveSubagentBinding(config, secondaryModelFlags(), own, undefined, permissiveModelCatalog)).toEqual({ model: 'provider/fast', thinking: 'low', }); - expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own, 'provider/smart')).toThrow( + expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own, 'provider/smart', permissiveModelCatalog)).toThrow( /Invalid model "provider\/smart"\. Available models: provider\/fast, primary\./, ); @@ -1973,7 +1979,7 @@ describe('subagent config section', () => { '[secondary_model]\nmodel = "provider/slow"\ndefault_model = "provider/fast"\n', ); - expect(resolveSubagentBinding(config, secondaryModelFlags(), own)).toEqual({ + expect(resolveSubagentBinding(config, secondaryModelFlags(), own, undefined, permissiveModelCatalog)).toEqual({ model: 'provider/fast', thinking: undefined, }); @@ -1981,16 +1987,17 @@ describe('subagent config section', () => { disposables.dispose(); }); - it('does not let the legacy model key substitute for a pool table default_model', async () => { + it('falls back to the first pool entry when a pool table has no default_model', async () => { const own = { modelAlias: 'provider/main', thinkingLevel: 'medium' }; const { config, disposables } = await createConfig( {}, '[secondary_model]\nmodel = "provider/fast"\n\n[secondary_model.models]\n"provider/fast" = "fast and cheap"\n', ); - expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own)).toThrow( - '[secondary_model].default_model is required when [secondary_model.models] is configured', - ); + expect(resolveSubagentBinding(config, secondaryModelFlags(), own, undefined, permissiveModelCatalog)).toEqual({ + model: 'provider/fast', + thinking: undefined, + }); disposables.dispose(); }); @@ -2002,11 +2009,11 @@ describe('subagent config section', () => { '[secondary_model]\nmodel = "provider/fast"\nforce = true\n', ); - expect(resolveSubagentBinding(config, secondaryModelFlags(), own)).toEqual({ + expect(resolveSubagentBinding(config, secondaryModelFlags(), own, undefined, permissiveModelCatalog)).toEqual({ model: 'provider/fast', thinking: undefined, }); - expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own, 'primary')).toThrow( + expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own, 'primary', permissiveModelCatalog)).toThrow( /Invalid model "primary": \[secondary_model\]\.force is set/, ); @@ -2044,11 +2051,11 @@ describe('subagent config section', () => { '[secondary_model]\ndefault_model = "provider/fast"\ndefault_effort = "max"\n', ); - expect(resolveSubagentBinding(config, secondaryModelFlags(), own)).toEqual({ + expect(resolveSubagentBinding(config, secondaryModelFlags(), own, undefined, permissiveModelCatalog)).toEqual({ model: 'provider/fast', thinking: 'max', }); - expect(resolveSubagentBinding(config, secondaryModelFlags(), own, 'primary')).toEqual({ + expect(resolveSubagentBinding(config, secondaryModelFlags(), own, 'primary', permissiveModelCatalog)).toEqual({ model: 'provider/main', thinking: 'medium', }); @@ -2067,25 +2074,25 @@ describe('subagent config section', () => { defaultModel: 'provider/fast', force: true, }); - expect(resolveSubagentBinding(config, secondaryModelFlags(), own)).toEqual({ + expect(resolveSubagentBinding(config, secondaryModelFlags(), own, undefined, permissiveModelCatalog)).toEqual({ model: 'provider/fast', thinking: undefined, }); - expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own, 'primary')).toThrow( + expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own, 'primary', permissiveModelCatalog)).toThrow( /Invalid model "primary": \[secondary_model\]\.force is set/, ); disposables.dispose(); }); - it('rejects force combined with a models table at spawn resolution, matching startup validation', async () => { + it('rejects force combined with a models table at spawn resolution', async () => { const own = { modelAlias: 'provider/main', thinkingLevel: 'medium' }; const { config, disposables } = await createConfig( {}, '[secondary_model]\ndefault_model = "provider/fast"\nforce = true\n\n[secondary_model.models]\n"provider/fast" = "fast and cheap"\n', ); - expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own)).toThrow( + expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own, undefined, permissiveModelCatalog)).toThrow( /\[secondary_model\]\.force cannot be combined with \[secondary_model\.models\]/, ); @@ -2101,7 +2108,7 @@ describe('subagent config section', () => { let caught: unknown; try { - resolveSubagentBinding(config, secondaryModelFlags(), own, 'provider/typo'); + resolveSubagentBinding(config, secondaryModelFlags(), own, 'provider/typo', permissiveModelCatalog); } catch (error) { caught = error; } @@ -2118,7 +2125,7 @@ describe('subagent config section', () => { const own = { modelAlias: 'provider/main', thinkingLevel: 'medium' }; const { config, disposables } = await createConfig({}); - expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own, 'provider/fast')).toThrow( + expect(() => resolveSubagentBinding(config, secondaryModelFlags(), own, 'provider/fast', permissiveModelCatalog)).toThrow( /Invalid model "provider\/fast": no \[secondary_model\.models\] pool is configured/, ); diff --git a/packages/agent-core-v2/test/features/swarm/swarm.test.ts b/packages/agent-core-v2/test/features/swarm/swarm.test.ts index 8386c91f0de..391cb405ac4 100644 --- a/packages/agent-core-v2/test/features/swarm/swarm.test.ts +++ b/packages/agent-core-v2/test/features/swarm/swarm.test.ts @@ -205,6 +205,22 @@ const SWARM_MODEL_ALIASES: ReadonlySet = new Set([ 'provider/smart', ]); +function stubSwarmModelCatalog(): IModelCatalog { + return { + _serviceBrand: undefined, + get: (alias: string) => { + if (!SWARM_MODEL_ALIASES.has(alias)) { + throw new Error2( + ErrorCodes.CONFIG_INVALID, + `Model "${alias}" is not configured in config.toml.`, + { details: { model: alias } }, + ); + } + return { id: alias } as Model; + }, + } as unknown as IModelCatalog; +} + function realSubagents( catalog: ISessionAgentProfileCatalog, config: IConfigService, @@ -250,19 +266,7 @@ function realSubagents( remove: async () => {}, broadcastPermissionMode: () => {}, } as unknown as IAgentLifecycleService; - const modelCatalog = { - _serviceBrand: undefined, - get: (alias: string) => { - if (!SWARM_MODEL_ALIASES.has(alias)) { - throw new Error2( - ErrorCodes.CONFIG_INVALID, - `Model "${alias}" is not configured in config.toml.`, - { details: { model: alias } }, - ); - } - return { id: alias } as Model; - }, - } as unknown as IModelCatalog; + const modelCatalog = stubSwarmModelCatalog(); const sessionContext = { _serviceBrand: undefined, cwd: '/repo' } as unknown as ISessionContext; return new SessionSubagentService( agentLifecycle, @@ -705,7 +709,7 @@ describe('AgentSwarmTool', () => { ]), }); const swarmMode = mockSwarmMode(); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), swarmMode, stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' } }), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' } }), stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), swarmMode, stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' } }), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' } }), stubFlag(true), stubCallerProfile()), stubCallerProfile()); const input = { description: 'Review files', prompt_template: 'Review {{item}}', @@ -804,7 +808,7 @@ describe('AgentSwarmTool', () => { it('does not expose permission rule argument matching', () => { const host = mockSwarmHost(); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); const execution = tool.resolveExecution({ description: 'Review files', prompt_template: 'Review {{item}}', @@ -819,7 +823,7 @@ describe('AgentSwarmTool', () => { it('description documents the {{item}} placeholder', () => { const host = mockSwarmHost(); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); expect(tool.description).toContain('{{item}}'); }); @@ -837,6 +841,7 @@ describe('AgentSwarmTool', () => { mockSwarmMode(), stubConfig(), stubFlag(true), + stubSwarmModelCatalog(), realSubagents( stubSwarmCatalog(caller), stubConfig(), @@ -907,7 +912,7 @@ describe('AgentSwarmTool', () => { for (const testCase of cases) { const host = mockSwarmHost(); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); const result = await executeTool(tool, context(testCase.input)); @@ -940,7 +945,7 @@ describe('AgentSwarmTool', () => { async ({ agentId }: { readonly agentId: string }) => persistedItems[agentId], ); const host = mockSwarmHost({ run, getSwarmItem }); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); const input = { description: 'Finish review', subagent_type: 'explore', @@ -1061,7 +1066,7 @@ describe('AgentSwarmTool', () => { ); const getSwarmItem = vi.fn(async () => 'src/old-a.ts'); const host = mockSwarmHost({ run, getSwarmItem }); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); const input = { description: 'Resume review', resume_agent_ids: { @@ -1124,7 +1129,7 @@ describe('AgentSwarmTool', () => { }, ]), }); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); const result = await executeTool( tool, @@ -1150,7 +1155,7 @@ describe('AgentSwarmTool', () => { it('passes the configured swarm timeout to swarm tasks', async () => { const host = mockSwarmHost(); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig({ timeoutMs: 5_000 }), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig({ timeoutMs: 5_000 }), stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig({ timeoutMs: 5_000 }), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig({ timeoutMs: 5_000 }), stubFlag(true), stubCallerProfile()), stubCallerProfile()); await executeTool( tool, @@ -1178,7 +1183,7 @@ describe('AgentSwarmTool', () => { get: (section: string) => section === SWARM_SECTION ? { timeoutMs: 5_000 } : { timeoutMs: 1_000 }, } as unknown as IConfigService; - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), sectionAwareConfig, stubFlag(true), realSubagents(stubSwarmCatalog(), sectionAwareConfig, stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), sectionAwareConfig, stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), sectionAwareConfig, stubFlag(true), stubCallerProfile()), stubCallerProfile()); await executeTool( tool, @@ -1201,7 +1206,7 @@ describe('AgentSwarmTool', () => { it('resolves spawn task plans from the configured model pool default', async () => { const host = mockSwarmHost(); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap', 'provider/smart': 'hard tasks' } }), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap', 'provider/smart': 'hard tasks' } }), stubFlag(true), stubCallerProfile({ modelAlias: 'main-model', thinkingLevel: 'high' })), stubCallerProfile({ modelAlias: 'main-model', thinkingLevel: 'high' })); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap', 'provider/smart': 'hard tasks' } }), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap', 'provider/smart': 'hard tasks' } }), stubFlag(true), stubCallerProfile({ modelAlias: 'main-model', thinkingLevel: 'high' })), stubCallerProfile({ modelAlias: 'main-model', thinkingLevel: 'high' })); await executeTool( tool, @@ -1228,7 +1233,7 @@ describe('AgentSwarmTool', () => { it('lets the tool call opt back into the primary model', async () => { const host = mockSwarmHost(); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' } }), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' } }), stubFlag(true), stubCallerProfile({ modelAlias: 'main-model', thinkingLevel: 'high' })), stubCallerProfile({ modelAlias: 'main-model', thinkingLevel: 'high' })); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' } }), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' } }), stubFlag(true), stubCallerProfile({ modelAlias: 'main-model', thinkingLevel: 'high' })), stubCallerProfile({ modelAlias: 'main-model', thinkingLevel: 'high' })); await executeTool( tool, @@ -1256,14 +1261,14 @@ describe('AgentSwarmTool', () => { it('advertises the configured pool in the description only when configured', async () => { const host = mockSwarmHost(); - const configured = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap', 'main-model': 'the main model' } }), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap', 'main-model': 'the main model' } }), stubFlag(true), stubCallerProfile({ modelAlias: 'main-model' })), stubCallerProfile({ modelAlias: 'main-model' })); + const configured = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap', 'main-model': 'the main model' } }), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap', 'main-model': 'the main model' } }), stubFlag(true), stubCallerProfile({ modelAlias: 'main-model' })), stubCallerProfile({ modelAlias: 'main-model' })); expect(configured.description).toContain('Available models'); expect(configured.description).toContain('- provider/fast [default]: fast and cheap'); expect(configured.description).toContain('- main-model [main model]: the main model'); expect(configured.description).toContain('- primary (main-model)'); - const unconfigured = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile({ modelAlias: 'main-model' })), stubCallerProfile({ modelAlias: 'main-model' })); + const unconfigured = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile({ modelAlias: 'main-model' })), stubCallerProfile({ modelAlias: 'main-model' })); expect(unconfigured.description).not.toContain('Available models'); }); @@ -1283,7 +1288,7 @@ describe('AgentSwarmTool', () => { }, ]), }); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); const result = await executeTool( tool, @@ -1330,7 +1335,7 @@ describe('AgentSwarmTool', () => { }, ]), }); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); const result = await executeTool( tool, @@ -1357,7 +1362,7 @@ describe('AgentSwarmTool', () => { it('rejects fork combined with resume_agent_ids', async () => { const host = mockSwarmHost(); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), stubCallerProfile()), stubCallerProfile()); const result = await executeTool( tool, @@ -1375,7 +1380,7 @@ describe('AgentSwarmTool', () => { it('rejects fork with a different subagent type', async () => { const host = mockSwarmHost(); const callerProfile = stubCallerProfile({ profileName: 'orchestrator' }); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), callerProfile), callerProfile); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), callerProfile), callerProfile); const result = await executeTool( tool, @@ -1395,7 +1400,7 @@ describe('AgentSwarmTool', () => { it('rejects fork with a model override', async () => { const host = mockSwarmHost(); const callerProfile = stubCallerProfile({ profileName: 'orchestrator', modelAlias: 'main-model' }); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), callerProfile), callerProfile); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), callerProfile), callerProfile); const result = await executeTool( tool, @@ -1414,7 +1419,7 @@ describe('AgentSwarmTool', () => { it('rejects fork while the subagent_fork experimental flag is off', async () => { const host = mockSwarmHost(); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(false), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(false), stubCallerProfile()), stubCallerProfile()); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(false), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(false), stubCallerProfile()), stubCallerProfile()); const result = await executeTool( tool, @@ -1437,7 +1442,7 @@ describe('AgentSwarmTool', () => { modelAlias: 'main-model', thinkingLevel: 'high', }); - const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), callerProfile), callerProfile); + const tool = new AgentSwarmTool(host.swarmService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockSwarmMode(), stubConfig(), stubFlag(true), stubSwarmModelCatalog(), realSubagents(stubSwarmCatalog(), stubConfig(), stubFlag(true), callerProfile), callerProfile); const result = await executeTool( tool, diff --git a/packages/agent-core-v2/test/session/subagent/spawn.test.ts b/packages/agent-core-v2/test/session/subagent/spawn.test.ts index 0ba013a3acf..6e54f2c2e3f 100644 --- a/packages/agent-core-v2/test/session/subagent/spawn.test.ts +++ b/packages/agent-core-v2/test/session/subagent/spawn.test.ts @@ -304,12 +304,12 @@ describe('SessionSubagentService planSpawn and spawn', () => { expect(error.message).toBe('Caller agent has no model bound'); }); - it('wraps an unresolvable pool model with the secondary-model config hint', async () => { + it('wraps an unresolvable forced model with the secondary-model config hint', async () => { const svc = service( { [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/bad', - models: { 'provider/bad': 'broken' }, + force: true, }, }, true, @@ -322,6 +322,22 @@ describe('SessionSubagentService planSpawn and spawn', () => { expect(error.message).toContain('comes from [secondary_model.models]'); }); + it('inherits the caller model when every pool entry fails to resolve', async () => { + const svc = service( + { + [SECONDARY_MODEL_SECTION]: { + defaultModel: 'provider/bad', + models: { 'provider/bad': 'broken' }, + }, + }, + true, + ); + + const plan = await svc.planSpawn({ callerAgentId: CALLER_ID, profileName: 'coder' }); + + expect(plan.model).toBe('main-model'); + }); + it('passes [secondary_model].default_effort as the explicit subagent thinking', async () => { modelIds.add('provider/fast'); const svc = service( diff --git a/packages/agent-core-v2/test/session/subagent/subagentModelsValidation.test.ts b/packages/agent-core-v2/test/session/subagent/subagentModelsValidation.test.ts index b31a61a90af..43983ac8d2a 100644 --- a/packages/agent-core-v2/test/session/subagent/subagentModelsValidation.test.ts +++ b/packages/agent-core-v2/test/session/subagent/subagentModelsValidation.test.ts @@ -1,248 +1,348 @@ -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; - -import { SyncDescriptor } from '#/_base/di/descriptors'; -import { DisposableStore } from '#/_base/di/lifecycle'; -import { TestInstantiationService } from '#/_base/di/test'; -import { IConfigService } from '#/app/config/config'; -import { IFlagService } from '#/app/flag/flag'; -import { ErrorCodes, Error2, isError2 } from '#/errors'; -import { IModelCatalog, type Model } from '#/kosong/model/catalog'; +import { describe, expect, it } from 'vitest'; + +import { ErrorCodes, Error2 } from '#/errors'; +import type { IModelCatalog } from '#/kosong/model/catalog'; +import type { Model } from '#/kosong/model/catalog'; import { + resolveEffectiveSubagentModelPool, + resolveSubagentBinding, SECONDARY_MODEL_SECTION, SUBAGENT_SECTION, } from '#/session/subagent/configSection'; import { SECONDARY_MODEL_FLAG_ID } from '#/session/subagent/flag'; -import { ISessionSubagentModelsValidationService } from '#/session/subagent/subagentModelsValidation'; -import { SessionSubagentModelsValidationService } from '#/session/subagent/subagentModelsValidationService'; import { StubConfigService } from '../../kosong/stubs'; import { stubFlag } from '../../app/flag/stubs'; -describe('SessionSubagentModelsValidationService', () => { - let disposables: DisposableStore; - let ix: TestInstantiationService; - let modelIds: Set; - - beforeEach(() => { - disposables = new DisposableStore(); - ix = disposables.add(new TestInstantiationService()); - modelIds = new Set(); - }); - afterEach(() => { - disposables.dispose(); - }); - - function setup(configValues: Record, flagEnabled = true): void { - ix.stub(IConfigService, new StubConfigService(configValues)); - ix.stub(IFlagService, stubFlag((id) => flagEnabled && id === SECONDARY_MODEL_FLAG_ID)); - ix.stub(IModelCatalog, { - _serviceBrand: undefined, - get: (id: string) => { - if (!modelIds.has(id)) { - throw new Error2( - ErrorCodes.CONFIG_INVALID, - `Model "${id}" is not configured in config.toml.`, - { details: { model: id } }, - ); - } - return { id } as Model; - }, - } as unknown as IModelCatalog); - ix.set( - ISessionSubagentModelsValidationService, - new SyncDescriptor(SessionSubagentModelsValidationService), - ); - } +function stubCatalog(modelIds: ReadonlySet): IModelCatalog { + return { + _serviceBrand: undefined, + get: (id: string) => { + if (!modelIds.has(id)) { + throw new Error2( + ErrorCodes.CONFIG_INVALID, + `Model "${id}" is not configured in config.toml.`, + { details: { model: id } }, + ); + } + return { id } as Model; + }, + } as unknown as IModelCatalog; +} - function resolve(): unknown { - try { - ix.get(ISessionSubagentModelsValidationService); - return undefined; - } catch (error) { - return error; - } - } +function resolvePool(configValues: Record, flagEnabled = true) { + return resolveEffectiveSubagentModelPool( + new StubConfigService(configValues), + stubFlag((id) => flagEnabled && id === SECONDARY_MODEL_FLAG_ID), + catalog, + ); +} +let catalog: IModelCatalog; + +function withModels(...ids: string[]): void { + catalog = stubCatalog(new Set(ids)); +} + +describe('resolveEffectiveSubagentModelPool', () => { it('is a no-op when no secondary_model section is configured', () => { - setup({}); - expect(resolve()).toBeUndefined(); + withModels(); + const { pool, issues } = resolvePool({}); + expect(pool).toBeUndefined(); + expect(issues).toEqual([]); }); it('is a no-op when only the [subagent] timeout is configured', () => { - setup({ [SUBAGENT_SECTION]: { timeoutMs: 5000 } }); - expect(resolve()).toBeUndefined(); + withModels(); + const { pool, issues } = resolvePool({ [SUBAGENT_SECTION]: { timeoutMs: 5000 } }); + expect(pool).toBeUndefined(); + expect(issues).toEqual([]); }); it('is a no-op for a broken pool while the secondary-model experiment is off', () => { - setup({ [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/typo' } }, false); - expect(resolve()).toBeUndefined(); - }); - - it('constructs fine when default_model alone forms an implicit single-entry pool', () => { - modelIds.add('provider/fast'); - setup({ [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/fast' } }); - expect(resolve()).toBeUndefined(); + withModels(); + const { pool, issues } = resolvePool( + { [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/typo' } }, + false, + ); + expect(pool).toBeUndefined(); + expect(issues).toEqual([]); }); - it('constructs fine when the legacy model key alone forms the fallback pool', () => { - modelIds.add('provider/fast'); - setup({ [SECONDARY_MODEL_SECTION]: { model: 'provider/fast' } }); - expect(resolve()).toBeUndefined(); + it('exposes an implicit single-entry pool for a resolvable lone default_model', () => { + withModels('provider/fast'); + const { pool, issues } = resolvePool({ + [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/fast' }, + }); + expect(pool).toEqual({ defaultModel: 'provider/fast', models: { 'provider/fast': '' } }); + expect(issues).toEqual([]); }); - it('fails session creation when the legacy model fallback does not resolve', () => { - setup({ [SECONDARY_MODEL_SECTION]: { model: 'provider/typo' } }); - const error = resolve(); - expect(isError2(error)).toBe(true); - expect((error as Error2).code).toBe(ErrorCodes.CONFIG_INVALID); - expect((error as Error2).message).toContain( - '[secondary_model.models] entry "provider/typo" could not be resolved', - ); + it('exposes the legacy model key as the fallback pool', () => { + withModels('provider/fast'); + const { pool, issues } = resolvePool({ + [SECONDARY_MODEL_SECTION]: { model: 'provider/fast' }, + }); + expect(pool).toEqual({ defaultModel: 'provider/fast', models: { 'provider/fast': '' } }); + expect(issues).toEqual([]); }); - it('constructs fine when force pins the legacy model fallback', () => { - modelIds.add('provider/fast'); - setup({ [SECONDARY_MODEL_SECTION]: { model: 'provider/fast', force: true } }); - expect(resolve()).toBeUndefined(); + it('drops an unresolvable legacy model fallback with a warning instead of failing', () => { + withModels(); + const { pool, issues } = resolvePool({ + [SECONDARY_MODEL_SECTION]: { model: 'provider/typo' }, + }); + expect(pool).toBeUndefined(); + expect(issues).toHaveLength(1); + expect(issues[0]).toContain('[secondary_model.models] entry "provider/typo" could not be resolved'); + expect(issues[0]).toContain('"provider/typo" is not configured'); + expect(issues[0]).toContain('The entry is ignored until fixed.'); }); - it('fails session creation when a pool table relies on the legacy model key for its default', () => { - modelIds.add('provider/fast'); - setup({ + it('passes a fully valid pool through unchanged', () => { + withModels('provider/fast', 'provider/smart'); + const { pool, issues } = resolvePool({ [SECONDARY_MODEL_SECTION]: { - model: 'provider/fast', - models: { 'provider/fast': 'fast and cheap' }, + defaultModel: 'provider/fast', + models: { 'provider/fast': 'fast and cheap', 'provider/smart': 'hard tasks' }, }, }); - const error = resolve(); - expect(isError2(error)).toBe(true); - expect((error as Error2).code).toBe(ErrorCodes.CONFIG_INVALID); - expect((error as Error2).message).toContain( - '[secondary_model].default_model is required when [secondary_model.models] is configured', - ); + expect(pool).toEqual({ + defaultModel: 'provider/fast', + models: { 'provider/fast': 'fast and cheap', 'provider/smart': 'hard tasks' }, + }); + expect(issues).toEqual([]); }); - it('fails session creation when a pool-less default_model does not resolve', () => { - setup({ [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/typo' } }); - const error = resolve(); - expect(isError2(error)).toBe(true); - expect((error as Error2).code).toBe(ErrorCodes.CONFIG_INVALID); - expect((error as Error2).message).toContain( - '[secondary_model.models] entry "provider/typo" could not be resolved', + it('falls back to the first resolvable entry when the pool has no default_model', () => { + withModels('provider/fast'); + const { pool, issues } = resolvePool({ + [SECONDARY_MODEL_SECTION]: { models: { 'provider/fast': 'fast and cheap' } }, + }); + expect(pool).toEqual({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' } }); + expect(issues).toHaveLength(1); + expect(issues[0]).toContain( + '[secondary_model].default_model is required when [secondary_model.models] is configured', ); + expect(issues[0]).toContain('falling back to "provider/fast" until fixed.'); }); - it('constructs fine for a valid pool', () => { - modelIds.add('provider/fast').add('provider/smart'); - setup({ + it('falls back to the first resolvable entry when default_model is not a pool key', () => { + withModels('provider/fast', 'provider/smart'); + const { pool, issues } = resolvePool({ [SECONDARY_MODEL_SECTION]: { - defaultModel: 'provider/fast', + defaultModel: 'provider/typo', models: { 'provider/fast': 'fast and cheap', 'provider/smart': 'hard tasks' }, }, }); - expect(resolve()).toBeUndefined(); + expect(pool?.defaultModel).toBe('provider/fast'); + expect(issues).toHaveLength(1); + expect(issues[0]).toContain( + '[secondary_model].default_model "provider/typo" is not available; falling back to "provider/fast" until fixed.', + ); + }); + + it('reports an unusable default_model when the models table is empty', () => { + withModels('provider/fast'); + const { pool, issues } = resolvePool({ + [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/fast', models: {} }, + }); + expect(pool).toBeUndefined(); + expect(issues).toHaveLength(1); + expect(issues[0]).toContain( + '[secondary_model].default_model "provider/fast" is not a [secondary_model.models] key', + ); + expect(issues[0]).toContain("subagents inherit the caller's model until fixed."); }); - it('fails session creation when the pool has no default_model', () => { - modelIds.add('provider/fast'); - setup({ [SECONDARY_MODEL_SECTION]: { models: { 'provider/fast': 'fast and cheap' } } }); - const error = resolve(); - expect(isError2(error)).toBe(true); - expect((error as Error2).code).toBe(ErrorCodes.CONFIG_INVALID); - expect((error as Error2).message).toContain( + it('reports a missing default_model when the models table is empty', () => { + withModels(); + const { pool, issues } = resolvePool({ + [SECONDARY_MODEL_SECTION]: { models: {} }, + }); + expect(pool).toBeUndefined(); + expect(issues).toHaveLength(1); + expect(issues[0]).toContain( '[secondary_model].default_model is required when [secondary_model.models] is configured', ); + expect(issues[0]).toContain("subagents inherit the caller's model until fixed."); }); - it('fails session creation when default_model is not a pool key, listing the pool', () => { - modelIds.add('provider/fast').add('provider/smart'); - setup({ - [SECONDARY_MODEL_SECTION]: { - defaultModel: 'provider/typo', - models: { 'provider/fast': 'fast and cheap', 'provider/smart': 'hard tasks' }, - }, + it('reports both the broken entry and the missing default when nothing survives', () => { + withModels(); + const { pool, issues } = resolvePool({ + [SECONDARY_MODEL_SECTION]: { models: { 'provider/typo': 'broken' } }, }); - const error = resolve(); - expect(isError2(error)).toBe(true); - expect((error as Error2).code).toBe(ErrorCodes.CONFIG_INVALID); - expect((error as Error2).message).toContain('"provider/typo"'); - expect((error as Error2).message).toContain( - 'Available models: provider/fast, provider/smart.', + expect(pool).toBeUndefined(); + expect(issues).toHaveLength(2); + expect(issues[0]).toContain('[secondary_model.models] entry "provider/typo" could not be resolved'); + expect(issues[1]).toContain( + '[secondary_model].default_model is required when [secondary_model.models] is configured', ); }); - it('fails session creation when a pool key uses the reserved "primary" alias', () => { - modelIds.add('primary').add('provider/fast'); - setup({ + it('drops the reserved "primary" alias with a warning and keeps the rest', () => { + withModels('primary', 'provider/fast'); + const { pool, issues } = resolvePool({ [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/fast', models: { primary: 'looks like a model', 'provider/fast': 'fast and cheap' }, }, }); - const error = resolve(); - expect(isError2(error)).toBe(true); - expect((error as Error2).code).toBe(ErrorCodes.CONFIG_INVALID); - expect((error as Error2).message).toContain( - '[secondary_model.models] key "primary" is reserved', - ); + expect(pool).toEqual({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' } }); + expect(issues).toHaveLength(1); + expect(issues[0]).toContain('[secondary_model.models] key "primary" is reserved'); + expect(issues[0]).toContain('The entry is ignored until renamed.'); }); - it('fails session creation when a pool key does not resolve, naming the key', () => { - modelIds.add('provider/fast'); - setup({ + it('skips an unresolvable pool entry, keeping the valid default and entries', () => { + withModels('provider/fast'); + const { pool, issues } = resolvePool({ [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap', 'provider/typo': 'hard tasks' }, }, }); - const error = resolve(); - expect(isError2(error)).toBe(true); - expect((error as Error2).code).toBe(ErrorCodes.CONFIG_INVALID); - expect((error as Error2).message).toContain( - '[secondary_model.models] entry "provider/typo" could not be resolved', + expect(pool).toEqual({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' } }); + expect(issues).toHaveLength(1); + expect(issues[0]).toContain('[secondary_model.models] entry "provider/typo" could not be resolved'); + expect(issues[0]).toContain('"provider/typo" is not configured'); + }); + + it('drops the whole pool when every entry fails to resolve', () => { + withModels(); + const { pool, issues } = resolvePool({ + [SECONDARY_MODEL_SECTION]: { + defaultModel: 'provider/typo', + models: { 'provider/typo': 'hard tasks', 'provider/gone': 'also broken' }, + }, + }); + expect(pool).toBeUndefined(); + expect(issues).toHaveLength(2); + expect(issues[0]).toContain('"provider/typo"'); + expect(issues[1]).toContain('"provider/gone"'); + }); + + it('falls back to a remaining entry when default_model itself fails to resolve', () => { + withModels('provider/smart'); + const { pool, issues } = resolvePool({ + [SECONDARY_MODEL_SECTION]: { + defaultModel: 'provider/typo', + models: { 'provider/typo': 'hard tasks', 'provider/smart': 'still here' }, + }, + }); + expect(pool).toEqual({ defaultModel: 'provider/smart', models: { 'provider/smart': 'still here' } }); + expect(issues).toHaveLength(2); + expect(issues[0]).toContain('[secondary_model.models] entry "provider/typo" could not be resolved'); + expect(issues[1]).toContain( + '[secondary_model].default_model "provider/typo" is not available; falling back to "provider/smart" until fixed.', ); - expect((error as Error2).message).toContain('"provider/typo" is not configured'); - expect(isError2((error as Error2).cause)).toBe(true); }); - it('constructs fine when force pins a resolvable default_model', () => { - modelIds.add('provider/fast'); - setup({ [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/fast', force: true } }); - expect(resolve()).toBeUndefined(); + it('reports no issues when force pins a resolvable default_model', () => { + withModels('provider/fast'); + const { pool, issues } = resolvePool({ + [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/fast', force: true }, + }); + expect(pool).toBeUndefined(); + expect(issues).toEqual([]); }); - it('fails session creation when force is set without default_model', () => { - setup({ [SECONDARY_MODEL_SECTION]: { force: true } }); - const error = resolve(); - expect(isError2(error)).toBe(true); - expect((error as Error2).code).toBe(ErrorCodes.CONFIG_INVALID); - expect((error as Error2).message).toContain( + it('warns instead of failing when force is set without default_model', () => { + withModels(); + const { issues } = resolvePool({ [SECONDARY_MODEL_SECTION]: { force: true } }); + expect(issues).toHaveLength(1); + expect(issues[0]).toContain( '[secondary_model].default_model is required when [secondary_model].force is set', ); }); - it('fails session creation when force is combined with a models table', () => { - modelIds.add('provider/fast'); - setup({ + it('warns instead of failing when force is combined with a models table', () => { + withModels('provider/fast'); + const { issues } = resolvePool({ [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap' }, force: true, }, }); - const error = resolve(); - expect(isError2(error)).toBe(true); - expect((error as Error2).code).toBe(ErrorCodes.CONFIG_INVALID); - expect((error as Error2).message).toContain( + expect(issues).toHaveLength(1); + expect(issues[0]).toContain( '[secondary_model].force cannot be combined with [secondary_model.models]', ); }); - it('fails session creation when the forced default_model does not resolve', () => { - setup({ [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/typo', force: true } }); - const error = resolve(); - expect(isError2(error)).toBe(true); - expect((error as Error2).code).toBe(ErrorCodes.CONFIG_INVALID); - expect((error as Error2).message).toContain('"provider/typo"'); + it('warns instead of failing when the forced default_model does not resolve', () => { + withModels(); + const { issues } = resolvePool({ + [SECONDARY_MODEL_SECTION]: { defaultModel: 'provider/typo', force: true }, + }); + expect(issues).toHaveLength(1); + expect(issues[0]).toContain('[secondary_model] forced model "provider/typo" could not be resolved'); + expect(issues[0]).toContain('Subagent spawns will fail until this is fixed.'); + }); +}); + +describe('resolveSubagentBinding over the effective pool', () => { + const own = { modelAlias: 'provider/main', thinkingLevel: 'medium' }; + + function bind(configValues: Record, requested?: string) { + return resolveSubagentBinding( + new StubConfigService(configValues), + stubFlag((id) => id === SECONDARY_MODEL_FLAG_ID), + own, + requested, + catalog, + ); + } + + it('rejects an explicit request for a skipped entry, listing only resolvable models', () => { + withModels('provider/fast'); + const config = { + [SECONDARY_MODEL_SECTION]: { + defaultModel: 'provider/fast', + models: { 'provider/fast': 'fast and cheap', 'provider/typo': 'hard tasks' }, + }, + }; + expect(() => bind(config, 'provider/typo')).toThrow( + 'Invalid model "provider/typo". Available models: provider/fast, primary.', + ); + }); + + it('inherits the caller model when every pool entry is skipped', () => { + withModels(); + const config = { + [SECONDARY_MODEL_SECTION]: { + defaultModel: 'provider/typo', + models: { 'provider/typo': 'hard tasks' }, + }, + }; + expect(bind(config)).toEqual({ model: 'provider/main', thinking: 'medium' }); + expect(() => bind(config, 'provider/typo')).toThrow( + /no \[secondary_model\.models\] pool is configured/, + ); + }); + + it('binds the fallback default when default_model is skipped', () => { + withModels('provider/smart'); + const config = { + [SECONDARY_MODEL_SECTION]: { + defaultModel: 'provider/typo', + models: { 'provider/typo': 'hard tasks', 'provider/smart': 'still here' }, + }, + }; + expect(bind(config)).toEqual({ model: 'provider/smart', thinking: undefined }); + }); + + it('binds "primary" to the caller model even when the raw pool abuses the reserved alias', () => { + withModels('primary', 'provider/fast'); + const config = { + [SECONDARY_MODEL_SECTION]: { + defaultModel: 'provider/fast', + models: { primary: 'looks like a model', 'provider/fast': 'fast and cheap' }, + }, + }; + expect(bind(config, 'primary')).toEqual({ model: 'provider/main', thinking: 'medium' }); + expect(bind(config)).toEqual({ model: 'provider/fast', thinking: undefined }); }); }); diff --git a/packages/agent-core-v2/test/tool/tool.test.ts b/packages/agent-core-v2/test/tool/tool.test.ts index 64866292293..6338d4b0fea 100644 --- a/packages/agent-core-v2/test/tool/tool.test.ts +++ b/packages/agent-core-v2/test/tool/tool.test.ts @@ -1896,7 +1896,7 @@ describe('Agent tool execution contract', () => { ); }); - it('rejects a pool that gained the reserved "primary" key through a runtime config edit', async () => { + it('inherits the caller model when a runtime config edit leaves only the reserved "primary" key', async () => { const lifecycle = createAgentLifecycleStub({ createAgentIds: ['agent-child'] }); const context = createAgentToolContext(lifecycle, secondaryModelFlags(), { initialConfig: { @@ -1916,9 +1916,12 @@ describe('Agent tool execution contract', () => { description: 'Find cause', }); - expect(result.isError).toBe(true); - expect(result.output).toContain('[secondary_model.models] key "primary" is reserved'); - expect(lifecycle.create).not.toHaveBeenCalled(); + expect(result.isError).toBeUndefined(); + expect(lifecycle.create).toHaveBeenCalledWith( + expect.objectContaining({ + binding: expect.objectContaining({ model: 'mock-model' }), + }), + ); }); it('points at the [secondary_model.models] config when the bound alias stops resolving', async () => { diff --git a/packages/kap-server/src/routes/sessions.ts b/packages/kap-server/src/routes/sessions.ts index 7230c2cdfbc..bd62a65f52b 100644 --- a/packages/kap-server/src/routes/sessions.ts +++ b/packages/kap-server/src/routes/sessions.ts @@ -14,7 +14,10 @@ import { ISessionMetadata, ISessionLegacyService, ISessionTitleService, + IConfigService, IEventService, + IFlagService, + IModelCatalog, SessionCreated, IWorkspaceAliases, ISessionManager, @@ -32,6 +35,7 @@ import { type SessionSummary, } from '@moonshot-ai/agent-core-v2'; import { SessionMetaUpdated } from '@moonshot-ai/agent-core-v2/session/sessionMetadata/sessionMetaEvents'; +import { resolveEffectiveSubagentModelPool } from '@moonshot-ai/agent-core-v2/session/subagent/configSection'; import { ErrorCode } from '../protocol/error-codes'; import { pageResponseSchema } from '../protocol/pagination'; import { toProtocolMessage } from '../services/messages/messageProjection'; @@ -806,7 +810,7 @@ export function registerSessionsRoutes(app: SessionRouteHost, core: Scope): void [ErrorCode.VALIDATION_FAILED]: { detailsSchema }, [ErrorCode.SESSION_NOT_FOUND]: {}, }, - description: 'Get session-level warnings (e.g. oversized AGENTS.md)', + description: 'Get session-level warnings (e.g. oversized AGENTS.md, broken secondary-model pool entries)', tags: ['sessions'], }, async (req, reply) => { @@ -831,6 +835,18 @@ export function registerSessionsRoutes(app: SessionRouteHost, core: Scope): void severity: 'warning' as const, }, ]; + const { issues } = resolveEffectiveSubagentModelPool( + core.accessor.get(IConfigService), + core.accessor.get(IFlagService), + core.accessor.get(IModelCatalog), + ); + for (const issue of issues) { + warnings.push({ + code: 'secondary-model-invalid', + message: issue, + severity: 'warning' as const, + }); + } reply.send(okEnvelope({ warnings }, req.id)); } catch (error) { sendMappedError(reply, req, error); diff --git a/packages/kap-server/test/config.test.ts b/packages/kap-server/test/config.test.ts index 61ba89503a0..8cc4ef81ba0 100644 --- a/packages/kap-server/test/config.test.ts +++ b/packages/kap-server/test/config.test.ts @@ -11,7 +11,6 @@ import { type Scope, } from '@moonshot-ai/agent-core-v2'; import { configResponseSchema, type ConfigResponse } from '../src/protocol/rest-config'; -import { ErrorCode } from '../src/protocol/error-codes'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { WebSocket } from 'ws'; @@ -159,7 +158,7 @@ describe('server-v2 /api/v1/config', () => { }); }); - it('session create with a broken subagent model pool fails with VALIDATION_FAILED', async () => { + it('session create with a broken subagent model pool succeeds and reports a startup warning', async () => { await boot( '[experimental]\n"secondary-model" = true\n\n[secondary_model.models]\n"provider/fast" = "fast and cheap"\n', ); @@ -168,9 +167,27 @@ describe('server-v2 /api/v1/config', () => { headers: { 'content-type': 'application/json' }, body: JSON.stringify({ metadata: { cwd: home as string } }), }); - const body = (await res.json()) as Envelope; - expect(body.code).toBe(ErrorCode.VALIDATION_FAILED); - expect(body.msg).toContain('[secondary_model].default_model is required'); + const body = (await res.json()) as Envelope<{ id: string }>; + expect(body.code).toBe(0); + + const warningsRes = await authedFetch( + server as RunningServer, + base, + `/api/v1/sessions/${body.data!.id}/warnings`, + ); + const warningsBody = (await warningsRes.json()) as Envelope<{ + warnings: { code: string; message: string }[]; + }>; + const secondary = warningsBody.data!.warnings.filter( + (warning) => warning.code === 'secondary-model-invalid', + ); + expect(secondary).toHaveLength(2); + expect(secondary[0]!.message).toContain( + '[secondary_model.models] entry "provider/fast" could not be resolved', + ); + expect(secondary[1]!.message).toContain( + '[secondary_model].default_model is required when [secondary_model.models] is configured', + ); }); it('session create with a broken subagent model pool succeeds while the experiment is off', async () => { diff --git a/packages/node-sdk/src/sdk-rpc-client-v2.ts b/packages/node-sdk/src/sdk-rpc-client-v2.ts index 54f9d015b35..70877d55c61 100644 --- a/packages/node-sdk/src/sdk-rpc-client-v2.ts +++ b/packages/node-sdk/src/sdk-rpc-client-v2.ts @@ -151,6 +151,7 @@ import { import { encodeWorkDirKey } from '@moonshot-ai/agent-core-v2/_base/utils/workdir-slug'; import { McpConnectionManager } from '@moonshot-ai/agent-core-v2/mcpCore/connection-manager'; import { loadMcpServers } from '@moonshot-ai/agent-core-v2/app/mcpConfig/configLoader'; +import { resolveEffectiveSubagentModelPool } from '@moonshot-ai/agent-core-v2/session/subagent/configSection'; import { IAppendLogStore } from '@moonshot-ai/agent-core-v2/persistence/interface/appendLogStore'; import type { McpServerConfig as WorkspaceMcpServerConfig } from '@moonshot-ai/agent-core-v2/mcpCore/config-schema'; import { @@ -186,10 +187,12 @@ import { IBootstrapService, IConfigService, IEventService, + IFlagService, IHostEnvironment, IHostFileSystem, IMcpManagementService, IMcpOAuthService, + IModelCatalog, IModelService, IProviderService, ISessionBtwService, @@ -2006,7 +2009,10 @@ export class SDKRpcClientV2 extends SDKRpcClientBase { * cache is empty — v1 recomputes on demand whenever no warning is cached, * so an AGENTS.md that outgrows the budget mid-session surfaces on both * engines. The single warning shape (`agents-md-oversized`, severity - * `warning`) mirrors v1's assembly. + * `warning`) mirrors v1's assembly. The `secondary-model-invalid` entries + * mirror v1's `computeSecondaryModelWarnings`: pool problems never block + * session startup, they surface here and the effective pool skips the + * broken entries until the config is fixed. */ override async getSessionWarnings(input: SessionIdRpcInput) { const agent = await this.agentScope(input.sessionId); @@ -2024,9 +2030,23 @@ export class SDKRpcClientV2 extends SDKRpcClientBase { ); warning = prepared.agentsMdWarning; } - return warning === undefined - ? [] - : [{ code: 'agents-md-oversized', message: warning, severity: 'warning' as const }]; + const warnings = + warning === undefined + ? [] + : [{ code: 'agents-md-oversized', message: warning, severity: 'warning' as const }]; + const { issues } = resolveEffectiveSubagentModelPool( + this.engineAccessor.get(IConfigService), + this.engineAccessor.get(IFlagService), + this.engineAccessor.get(IModelCatalog), + ); + for (const issue of issues) { + warnings.push({ + code: 'secondary-model-invalid', + message: issue, + severity: 'warning' as const, + }); + } + return warnings; } /**