diff --git a/.changeset/subagent-model-policy-endpoint.md b/.changeset/subagent-model-policy-endpoint.md new file mode 100644 index 000000000..ee955dee5 --- /dev/null +++ b/.changeset/subagent-model-policy-endpoint.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": minor +--- + +Add a subagent model policy setting with inherit, default, pool, and force modes that rejects models that are not configured. diff --git a/packages/agent-core-v2/src/app/config/config.ts b/packages/agent-core-v2/src/app/config/config.ts index 9003c1d36..f668acebc 100644 --- a/packages/agent-core-v2/src/app/config/config.ts +++ b/packages/agent-core-v2/src/app/config/config.ts @@ -205,6 +205,7 @@ export interface IConfigService { sections: Readonly>, target?: ConfigTarget, ): Promise; + previewReplaceSections(sections: Readonly>): ResolvedConfig; reload(): Promise; diagnostics(): readonly ConfigDiagnostic[]; } diff --git a/packages/agent-core-v2/src/app/config/configService.ts b/packages/agent-core-v2/src/app/config/configService.ts index a095c7687..add4ff8c7 100644 --- a/packages/agent-core-v2/src/app/config/configService.ts +++ b/packages/agent-core-v2/src/app/config/configService.ts @@ -487,6 +487,24 @@ export class ConfigService extends Disposable implements IConfigService { }); } + previewReplaceSections(sections: Readonly>): ResolvedConfig { + const stagedRaw: ResolvedConfig = { ...this.raw }; + const stagedRawSnake = cloneRecord(this.rawSnake); + for (const domain of Object.keys(sections)) { + const value = sections[domain] === null ? undefined : sections[domain]; + const stripped = this.stripEnv(domain, value, stagedRaw, stagedRawSnake); + if (stripped === undefined) { + delete stagedRaw[domain]; + } else { + stagedRaw[domain] = this.registry.validate(domain, stripped); + } + } + const next: ResolvedConfig = { ...this.buildValidated(stagedRaw, false) }; + this.applySectionEnvBindings(next, false); + this.applyEnvOverlay(next, false); + return { ...next, ...this.memory }; + } + private stripEnv( domain: string, value: unknown, @@ -599,12 +617,13 @@ export class ConfigService extends Disposable implements IConfigService { } } - private buildValidated(raw: ResolvedConfig): ResolvedConfig { + private buildValidated(raw: ResolvedConfig, report = true): ResolvedConfig { const validated: ResolvedConfig = {}; for (const [domain, value] of Object.entries(raw)) { try { validated[domain] = this.registry.validate(domain, value); } catch (error) { + if (!report) continue; this.pushDiagnostic({ domain, severity: 'warning', diff --git a/packages/agent-core-v2/src/app/config/errors.ts b/packages/agent-core-v2/src/app/config/errors.ts index 63c01cb81..518fda1c7 100644 --- a/packages/agent-core-v2/src/app/config/errors.ts +++ b/packages/agent-core-v2/src/app/config/errors.ts @@ -5,6 +5,7 @@ export const ConfigErrors = { codes: { CONFIG_INVALID: CONFIG_INVALID_ERROR_CODE, CONFIG_PERSIST_BLOCKED: 'config.persist_blocked', + CONFIG_VERSION_CONFLICT: 'config.version_conflict', }, } as const satisfies ErrorDomain; diff --git a/packages/agent-core-v2/src/app/kosongConfig/discoveryService.ts b/packages/agent-core-v2/src/app/kosongConfig/discoveryService.ts index b7363acc7..bc0360aae 100644 --- a/packages/agent-core-v2/src/app/kosongConfig/discoveryService.ts +++ b/packages/agent-core-v2/src/app/kosongConfig/discoveryService.ts @@ -25,9 +25,8 @@ import { PROVIDERS_SECTION, THINKING_SECTION, } from './configSection'; -import { - SECONDARY_MODEL_SECTION, -} from '#/session/subagent/configSection'; +import { prospectiveModelView, SECONDARY_MODEL_SECTION } from '#/session/subagent/policy'; +import { ISubagentModelPolicyService } from '#/session/subagent/subagentModelPolicy'; import { IProviderDiscoveryService, ModelCatalogChanged, @@ -54,6 +53,7 @@ export class ProviderDiscoveryService implements IProviderDiscoveryService { @IConfigService private readonly config: IConfigService, @IEventService private readonly events: IEventService, @IAgentIdentity private readonly identity: IAgentIdentity, + @ISubagentModelPolicyService private readonly subagentPolicy: ISubagentModelPolicyService, ) {} refreshProviderModels( @@ -216,7 +216,11 @@ export class ProviderDiscoveryService implements IProviderDiscoveryService { sections[THINKING_SECTION] = restoreDefault ? exclusion.thinking : patch.thinking; } if ('secondaryModel' in patch) { - sections[SECONDARY_MODEL_SECTION] = patch.secondaryModel; + const preview = this.config.previewReplaceSections(sections); + sections[SECONDARY_MODEL_SECTION] = this.subagentPolicy.prepareLegacyMutation( + patch.secondaryModel, + prospectiveModelView(preview[PROVIDERS_SECTION], preview[MODELS_SECTION]), + ).section; } await this.config.replaceSections(sections); return { diff --git a/packages/agent-core-v2/src/index.ts b/packages/agent-core-v2/src/index.ts index 288529bcd..5314ce562 100644 --- a/packages/agent-core-v2/src/index.ts +++ b/packages/agent-core-v2/src/index.ts @@ -484,6 +484,9 @@ export * from '#/session/subagent/spawn'; import '#/session/subagent/flag'; export * from '#/session/subagent/subagentModelsValidation'; import '#/session/subagent/subagentModelsValidationService'; +export * from '#/session/subagent/policy'; +export * from '#/session/subagent/subagentModelPolicy'; +import '#/session/subagent/subagentModelPolicyService'; 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 aee10fc21..13ae7fcca 100644 --- a/packages/agent-core-v2/src/session/subagent/configSection.ts +++ b/packages/agent-core-v2/src/session/subagent/configSection.ts @@ -18,9 +18,33 @@ import { } from '#/kosong/model/thinking'; import { SECONDARY_MODEL_FLAG_ID } from './flag'; +import { + type CanonicalSubagentModelPolicy, + INHERIT_SUBAGENT_MODEL_POLICY, + type LegacySecondaryModelConfig, + LegacySecondaryModelConfigSchema, + normalizeLegacySecondaryModel, + normalizeLegacySecondaryModelOrInherit, + PRIMARY_SUBAGENT_MODEL_CHOICE, + SECONDARY_MODEL_DEFAULT_MODEL_REQUIRED_MESSAGE, + SECONDARY_MODEL_FORCE_EXCLUDES_MODELS_MESSAGE, + SECONDARY_MODEL_FORCE_REQUIRES_DEFAULT_MESSAGE, + SECONDARY_MODEL_PRIMARY_MODEL_RESERVED_MESSAGE, + SECONDARY_MODEL_SECTION, + subagentPolicyModelChoices, + validateSubagentModelPolicy, +} from './policy'; + +export { + PRIMARY_SUBAGENT_MODEL_CHOICE, + SECONDARY_MODEL_DEFAULT_MODEL_REQUIRED_MESSAGE, + SECONDARY_MODEL_FORCE_EXCLUDES_MODELS_MESSAGE, + SECONDARY_MODEL_FORCE_REQUIRES_DEFAULT_MESSAGE, + SECONDARY_MODEL_PRIMARY_MODEL_RESERVED_MESSAGE, + SECONDARY_MODEL_SECTION, +}; export const SUBAGENT_SECTION = 'subagent'; -export const SECONDARY_MODEL_SECTION = 'secondaryModel'; export const SubagentConfigSchema = z.object({ timeoutMs: z.number().int().min(0).optional(), @@ -28,24 +52,9 @@ export const SubagentConfigSchema = z.object({ export type SubagentConfig = z.infer; -export const SecondaryModelConfigSchema = z.object({ - defaultModel: z.string().min(1).optional(), - models: z.record(z.string(), z.string()).optional(), - force: z.boolean().optional(), - model: z.string().min(1).optional(), - maxContextSize: z.number().int().min(1).optional(), - maxInputSize: z.number().int().min(1).optional(), - maxOutputSize: z.number().int().min(1).optional(), - capabilities: z.array(z.string()).optional(), - displayName: z.string().optional(), - reasoningKey: z.string().optional(), - adaptiveThinking: z.boolean().optional(), - supportEfforts: z.array(z.string()).optional(), - defaultEffort: z.string().optional(), - offEffort: z.string().optional(), -}); +export const SecondaryModelConfigSchema = LegacySecondaryModelConfigSchema; -export type SecondaryModelConfig = z.infer; +export type SecondaryModelConfig = LegacySecondaryModelConfig; export const DEFAULT_SUBAGENT_TIMEOUT_MS = 2 * 60 * 60 * 1000; @@ -71,7 +80,7 @@ registerConfigSection(SUBAGENT_SECTION, SubagentConfigSchema, { stripEnv: stripSubagentEnv, }); -registerConfigSection(SECONDARY_MODEL_SECTION, SecondaryModelConfigSchema); +registerConfigSection('secondaryModel', SecondaryModelConfigSchema); export function resolveSubagentTimeoutMs(config: IConfigService): number { return ( @@ -80,35 +89,32 @@ export function resolveSubagentTimeoutMs(config: IConfigService): number { ); } -export const PRIMARY_SUBAGENT_MODEL_CHOICE = 'primary'; - export interface SubagentModelPool { readonly defaultModel?: string; readonly models: Record; } -export function resolveSubagentModelPool(config: IConfigService): SubagentModelPool | undefined { - const section = config.get(SECONDARY_MODEL_SECTION); - if (section?.models !== undefined) { - return { defaultModel: section.defaultModel, models: section.models }; - } - if (section?.defaultModel !== undefined) { - return { defaultModel: section.defaultModel, models: { [section.defaultModel]: '' } }; - } - if (section?.model !== undefined) { - return { defaultModel: section.model, models: { [section.model]: '' } }; - } - return undefined; +function configuredPolicy(config: IConfigService): CanonicalSubagentModelPolicy { + return normalizeLegacySecondaryModel( + config.get(SECONDARY_MODEL_SECTION), + ); } -export const SECONDARY_MODEL_FORCE_REQUIRES_DEFAULT_MESSAGE = - '[secondary_model].default_model is required when [secondary_model].force is set'; +function configuredPolicyOrInherit(config: IConfigService): CanonicalSubagentModelPolicy { + return normalizeLegacySecondaryModelOrInherit( + config.get(SECONDARY_MODEL_SECTION), + ); +} -export const SECONDARY_MODEL_FORCE_EXCLUDES_MODELS_MESSAGE = - '[secondary_model].force cannot be combined with [secondary_model.models]: the pool table only exists to offer the main agent a choice, and force removes that choice'; +export function resolveSubagentModelPool(config: IConfigService): SubagentModelPool | undefined { + const policy = configuredPolicyOrInherit(config); + const models = subagentPolicyModelChoices(policy); + if (policy.mode === 'inherit' || models === undefined) return undefined; + return { defaultModel: policy.defaultModel, models: { ...models } }; +} export function isSubagentModelForced(config: IConfigService): boolean { - return config.get(SECONDARY_MODEL_SECTION)?.force === true; + return configuredPolicyOrInherit(config).mode === 'force'; } export function exposesSubagentModelChoice(config: IConfigService, flags: IFlagService): boolean { @@ -117,10 +123,14 @@ export function exposesSubagentModelChoice(config: IConfigService, flags: IFlagS 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.`; +function catalogValidationContext(modelCatalog: IModelCatalog) { + return { + resolveModel(alias: string) { + const model = modelCatalog.get(alias); + return { id: model.id, defaultEffort: model.defaultEffort, supportEfforts: model.supportEfforts }; + }, + }; +} export function assertValidSubagentModelPool( pool: SubagentModelPool, @@ -135,30 +145,15 @@ export function assertValidSubagentModelPool( }, }); } - 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 } }, - ); - } - for (const alias of aliases) { - try { - modelCatalog.get(alias); - } 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 } }, - ); - } - } + validateSubagentModelPolicy( + { mode: 'pool', defaultModel: pool.defaultModel, models: { ...pool.models } }, + catalogValidationContext(modelCatalog), + ); } export function assertValidSubagentModelConfig( @@ -167,21 +162,7 @@ export function assertValidSubagentModelConfig( 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); + validateSubagentModelPolicy(configuredPolicy(config), catalogValidationContext(modelCatalog)); } export function cascadeSubagentModelPool( @@ -215,33 +196,21 @@ export function resolveSubagentBinding( requested?: string, ): { model: string; thinking?: string } { const enabled = flags.enabled(SECONDARY_MODEL_FLAG_ID); - const section = config.get(SECONDARY_MODEL_SECTION); - if (enabled && 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' }, - }); - } - const forcedModel = section.defaultModel ?? section.model; - if (forcedModel === undefined) { - throw new Error2(ErrorCodes.CONFIG_INVALID, SECONDARY_MODEL_FORCE_REQUIRES_DEFAULT_MESSAGE, { - details: { section: SECONDARY_MODEL_SECTION, field: 'defaultModel' }, - }); - } + const policy = enabled ? configuredPolicy(config) : INHERIT_SUBAGENT_MODEL_POLICY; + if (policy.mode === 'force') { if (requested !== undefined) { throw new Error2( ErrorCodes.CONFIG_INVALID, - `Invalid model "${requested}": [secondary_model].force is set, so every subagent binds "${forcedModel}" (omit the model parameter).`, + `Invalid model "${requested}": [secondary_model].force is set, so every subagent binds "${policy.defaultModel}" (omit the model parameter).`, { details: { model: requested } }, ); } - return { model: forcedModel, thinking: section.defaultEffort }; + return { model: policy.defaultModel, thinking: policy.defaultEffort }; } if (requested === PRIMARY_SUBAGENT_MODEL_CHOICE) { return { model: own.modelAlias, thinking: own.thinkingLevel }; } - const pool = enabled ? resolveSubagentModelPool(config) : undefined; - if (pool === undefined) { + if (policy.mode === 'inherit') { if (requested !== undefined) { throw new Error2( ErrorCodes.CONFIG_INVALID, @@ -251,7 +220,8 @@ export function resolveSubagentBinding( } return { model: own.modelAlias, thinking: own.thinkingLevel }; } - if (Object.hasOwn(pool.models, PRIMARY_SUBAGENT_MODEL_CHOICE)) { + const choices = subagentPolicyModelChoices(policy) ?? {}; + if (Object.hasOwn(choices, PRIMARY_SUBAGENT_MODEL_CHOICE)) { throw new Error2(ErrorCodes.CONFIG_INVALID, SECONDARY_MODEL_PRIMARY_MODEL_RESERVED_MESSAGE, { details: { section: SECONDARY_MODEL_SECTION, @@ -260,21 +230,16 @@ export function resolveSubagentBinding( }, }); } - const choice = requested ?? pool.defaultModel; - if (choice === 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, choice)) { - const available = [...Object.keys(pool.models), PRIMARY_SUBAGENT_MODEL_CHOICE]; + const choice = requested ?? policy.defaultModel; + if (!Object.hasOwn(choices, choice)) { + const available = [...Object.keys(choices), PRIMARY_SUBAGENT_MODEL_CHOICE]; throw new Error2( ErrorCodes.CONFIG_INVALID, `Invalid model "${choice}". Available models: ${available.join(', ')}.`, { details: { model: choice, availableModels: available } }, ); } - return { model: choice, thinking: section?.defaultEffort }; + return { model: choice, thinking: policy.defaultEffort }; } export function resolveSubagentThinking( diff --git a/packages/agent-core-v2/src/session/subagent/policy.ts b/packages/agent-core-v2/src/session/subagent/policy.ts new file mode 100644 index 000000000..3e1c0fffd --- /dev/null +++ b/packages/agent-core-v2/src/session/subagent/policy.ts @@ -0,0 +1,347 @@ +import { createHash } from 'node:crypto'; + +import { z } from 'zod'; + +import { Error2, ErrorCodes } from '#/errors'; +import type { ExperimentalFlagSource } from '#/app/flag/flag'; +import { isPlainObject } from '#/app/config/configPure'; + +export const SECONDARY_MODEL_SECTION = 'secondaryModel'; + +export const PRIMARY_SUBAGENT_MODEL_CHOICE = 'primary'; + +export const LegacySecondaryModelConfigSchema = z.object({ + defaultModel: z.string().min(1).optional(), + models: z.record(z.string(), z.string()).optional(), + force: z.boolean().optional(), + model: z.string().min(1).optional(), + maxContextSize: z.number().int().min(1).optional(), + maxInputSize: z.number().int().min(1).optional(), + maxOutputSize: z.number().int().min(1).optional(), + capabilities: z.array(z.string()).optional(), + displayName: z.string().optional(), + reasoningKey: z.string().optional(), + adaptiveThinking: z.boolean().optional(), + supportEfforts: z.array(z.string()).optional(), + defaultEffort: z.string().optional(), + offEffort: z.string().optional(), +}); + +export type LegacySecondaryModelConfig = z.infer; + +const modelAliasSchema = z.string().min(1); +const effortSchema = z.string().min(1).optional(); + +export const CanonicalSubagentModelPolicySchema = z.discriminatedUnion('mode', [ + z.object({ mode: z.literal('inherit') }).strict(), + z + .object({ mode: z.literal('default'), defaultModel: modelAliasSchema, defaultEffort: effortSchema }) + .strict(), + z + .object({ + mode: z.literal('pool'), + defaultModel: modelAliasSchema, + models: z.record(z.string(), z.string()), + defaultEffort: effortSchema, + }) + .strict(), + z + .object({ mode: z.literal('force'), defaultModel: modelAliasSchema, defaultEffort: effortSchema }) + .strict(), +]); + +export type CanonicalSubagentModelPolicy = z.infer; + +export type SubagentModelPolicyMode = CanonicalSubagentModelPolicy['mode']; + +export type SubagentPolicySource = 'config' | 'default'; + +export const INHERIT_SUBAGENT_MODEL_POLICY: CanonicalSubagentModelPolicy = Object.freeze({ + mode: 'inherit', +}) as CanonicalSubagentModelPolicy; + +export const SECONDARY_MODEL_FORCE_REQUIRES_DEFAULT_MESSAGE = + '[secondary_model].default_model is required when [secondary_model].force is set'; + +export const SECONDARY_MODEL_FORCE_EXCLUDES_MODELS_MESSAGE = + '[secondary_model].force cannot be combined with [secondary_model.models]: the pool table only exists to offer the main agent a choice, and force removes that choice'; + +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.`; + +function invalid(message: string, details: Record): Error2 { + return new Error2(ErrorCodes.CONFIG_INVALID, message, { + details: { section: SECONDARY_MODEL_SECTION, ...details }, + }); +} + +function effortOf(defaultEffort: string | undefined): string | undefined { + return defaultEffort === undefined || defaultEffort.length === 0 ? undefined : defaultEffort; +} + +export function normalizeLegacySecondaryModel( + legacy: LegacySecondaryModelConfig | undefined, +): CanonicalSubagentModelPolicy { + if (legacy === undefined) return INHERIT_SUBAGENT_MODEL_POLICY; + const single = legacy.defaultModel ?? legacy.model; + if (legacy.force === true) { + if (legacy.models !== undefined) { + throw invalid(SECONDARY_MODEL_FORCE_EXCLUDES_MODELS_MESSAGE, { field: 'force' }); + } + if (single === undefined) { + throw invalid(SECONDARY_MODEL_FORCE_REQUIRES_DEFAULT_MESSAGE, { field: 'defaultModel' }); + } + return { mode: 'force', defaultModel: single, defaultEffort: effortOf(legacy.defaultEffort) }; + } + if (legacy.models !== undefined) { + if (legacy.defaultModel === undefined) { + throw invalid(SECONDARY_MODEL_DEFAULT_MODEL_REQUIRED_MESSAGE, { field: 'defaultModel' }); + } + return { + mode: 'pool', + defaultModel: legacy.defaultModel, + models: { ...legacy.models }, + defaultEffort: effortOf(legacy.defaultEffort), + }; + } + if (single !== undefined) { + return { mode: 'default', defaultModel: single, defaultEffort: effortOf(legacy.defaultEffort) }; + } + return INHERIT_SUBAGENT_MODEL_POLICY; +} + +export function normalizeLegacySecondaryModelOrInherit( + legacy: LegacySecondaryModelConfig | undefined, +): CanonicalSubagentModelPolicy { + try { + return normalizeLegacySecondaryModel(legacy); + } catch { + return INHERIT_SUBAGENT_MODEL_POLICY; + } +} + +export function toPersistedSecondaryModel( + policy: CanonicalSubagentModelPolicy, +): LegacySecondaryModelConfig | undefined { + switch (policy.mode) { + case 'inherit': + return undefined; + case 'default': + return { defaultModel: policy.defaultModel, defaultEffort: policy.defaultEffort }; + case 'pool': + return { + defaultModel: policy.defaultModel, + models: { ...policy.models }, + defaultEffort: policy.defaultEffort, + }; + case 'force': + return { defaultModel: policy.defaultModel, force: true, defaultEffort: policy.defaultEffort }; + } +} + +export function parseCanonicalSubagentModelPolicy(input: unknown): CanonicalSubagentModelPolicy { + const parsed = CanonicalSubagentModelPolicySchema.safeParse(input); + if (!parsed.success) { + const issue = parsed.error.issues[0]; + const path = issue === undefined || issue.path.length === 0 ? '' : `${issue.path.join('.')}: `; + throw invalid( + `Invalid subagent model policy: ${path}${issue?.message ?? 'malformed policy'}`, + { field: issue?.path.join('.') }, + ); + } + return parsed.data; +} + +export interface SubagentPolicyModelInfo { + readonly id: string; + readonly defaultEffort?: string; + readonly supportEfforts?: readonly string[]; +} + +export interface SubagentPolicyValidationContext { + resolveModel(alias: string): SubagentPolicyModelInfo | undefined; +} + +export function subagentPolicyModelChoices( + policy: CanonicalSubagentModelPolicy, +): Readonly> | undefined { + switch (policy.mode) { + case 'inherit': + return undefined; + case 'pool': + return policy.models; + case 'default': + case 'force': + return { [policy.defaultModel]: '' }; + } +} + +function assertModelResolves( + alias: string, + field: string, + context: SubagentPolicyValidationContext, +): SubagentPolicyModelInfo { + const label = + field === 'models' + ? `[secondary_model.models] entry "${alias}"` + : `[secondary_model].default_model "${alias}"`; + let info: SubagentPolicyModelInfo | undefined; + try { + info = context.resolveModel(alias); + } catch (error) { + throw new Error2( + ErrorCodes.CONFIG_INVALID, + `${label} could not be resolved: ${error instanceof Error ? error.message : String(error)}`, + { cause: error, details: { section: SECONDARY_MODEL_SECTION, field, model: alias } }, + ); + } + if (info === undefined) { + throw invalid(`${label} could not be resolved: Model "${alias}" is not configured in config.toml.`, { + field, + model: alias, + }); + } + return info; +} + +export function validateSubagentModelPolicy( + policy: CanonicalSubagentModelPolicy, + context: SubagentPolicyValidationContext, +): void { + if (policy.mode === 'inherit') return; + if (policy.mode === 'pool') { + if (Object.hasOwn(policy.models, PRIMARY_SUBAGENT_MODEL_CHOICE)) { + throw invalid(SECONDARY_MODEL_PRIMARY_MODEL_RESERVED_MESSAGE, { + field: 'models', + model: PRIMARY_SUBAGENT_MODEL_CHOICE, + }); + } + const aliases = Object.keys(policy.models); + if (!Object.hasOwn(policy.models, policy.defaultModel)) { + throw new Error2( + ErrorCodes.CONFIG_INVALID, + `[secondary_model].default_model "${policy.defaultModel}" is not a [secondary_model.models] key. Available models: ${aliases.join(', ')}.`, + { details: { model: policy.defaultModel, availableModels: aliases } }, + ); + } + for (const alias of aliases) assertModelResolves(alias, 'models', context); + } + const bound = assertModelResolves(policy.defaultModel, 'defaultModel', context); + if ( + policy.defaultEffort !== undefined && + bound.supportEfforts !== undefined && + bound.supportEfforts.length > 0 && + !bound.supportEfforts.includes(policy.defaultEffort) + ) { + throw invalid( + `[secondary_model].default_effort "${policy.defaultEffort}" is not supported by "${policy.defaultModel}". Supported efforts: ${bound.supportEfforts.join(', ')}.`, + { field: 'defaultEffort', model: policy.defaultModel, effort: policy.defaultEffort }, + ); + } +} + +export function prospectiveModelView(providers: unknown, models: unknown): SubagentPolicyValidationContext { + const providerTable = isPlainObject(providers) ? providers : {}; + const modelTable = isPlainObject(models) ? models : {}; + const resolveEntry = (alias: string): Record | undefined => { + const direct = modelTable[alias]; + if (isPlainObject(direct)) return direct; + for (const entry of Object.values(modelTable)) { + if (!isPlainObject(entry)) continue; + const aliases = entry['aliases']; + if (Array.isArray(aliases) && aliases.includes(alias)) return entry; + } + return undefined; + }; + return { + resolveModel(alias) { + const entry = resolveEntry(alias); + if (entry === undefined) return undefined; + const provider = entry['provider']; + if (typeof provider === 'string' && !isPlainObject(providerTable[provider])) return undefined; + const supportEfforts = entry['supportEfforts']; + const defaultEffort = entry['defaultEffort']; + return { + id: alias, + defaultEffort: typeof defaultEffort === 'string' ? defaultEffort : undefined, + supportEfforts: Array.isArray(supportEfforts) + ? supportEfforts.filter((effort): effort is string => typeof effort === 'string') + : undefined, + }; + }, + }; +} + +export function canonicalJson(value: unknown): string { + return JSON.stringify(sortKeys(value)); +} + +function sortKeys(value: unknown): unknown { + if (Array.isArray(value)) return value.map(sortKeys); + if (!isPlainObject(value)) return value === undefined ? null : value; + const out: Record = {}; + for (const key of Object.keys(value).toSorted()) { + const entry = value[key]; + if (entry === undefined) continue; + out[key] = sortKeys(entry); + } + return out; +} + +function digest(input: string): string { + return createHash('sha256').update(input, 'utf8').digest('hex').slice(0, 32); +} + +export const SUBAGENT_POLICY_RESOURCE_VERSION_PREFIX = 'subagent-policy-v1:'; + +export function subagentPolicyResourceVersion( + persisted: LegacySecondaryModelConfig | undefined, +): string { + let canonical: unknown; + try { + const policy = normalizeLegacySecondaryModel(persisted); + canonical = policy.mode === 'inherit' ? null : policy; + } catch { + canonical = { invalid: persisted ?? null }; + } + return `${SUBAGENT_POLICY_RESOURCE_VERSION_PREFIX}${digest(canonicalJson(canonical))}`; +} + +export interface SubagentFeatureState { + readonly enabled: boolean; + readonly source: ExperimentalFlagSource; +} + +export interface RoutingEnvironmentInput { + readonly effectivePolicy: CanonicalSubagentModelPolicy; + readonly policySource: SubagentPolicySource; + readonly feature: SubagentFeatureState; + readonly callerModel: string; + readonly callerThinking?: string; + readonly thinkingEnabled: boolean; + readonly boundModelDefaultEffort?: string; +} + +export const ROUTING_ENVIRONMENT_REVISION_PREFIX = 'route-env:v1:'; + +export function routingEnvironmentRevision(input: RoutingEnvironmentInput): string { + return `${ROUTING_ENVIRONMENT_REVISION_PREFIX}${digest(canonicalJson(input))}`; +} + +export type SubagentRoutingOperation = 'spawn' | 'fork' | 'resume'; + +export interface RouteDecisionInput { + readonly routingEnvironmentRevision: string; + readonly operation: SubagentRoutingOperation; + readonly profile?: string; + readonly model?: string; + readonly thinking?: string; +} + +export const ROUTE_DECISION_FINGERPRINT_PREFIX = 'route-decision:v1:'; + +export function routeDecisionFingerprint(input: RouteDecisionInput): string { + return `${ROUTE_DECISION_FINGERPRINT_PREFIX}${digest(canonicalJson(input))}`; +} diff --git a/packages/agent-core-v2/src/session/subagent/subagentModelPolicy.ts b/packages/agent-core-v2/src/session/subagent/subagentModelPolicy.ts new file mode 100644 index 000000000..ae80a7b8b --- /dev/null +++ b/packages/agent-core-v2/src/session/subagent/subagentModelPolicy.ts @@ -0,0 +1,48 @@ +import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation'; + +import type { + CanonicalSubagentModelPolicy, + LegacySecondaryModelConfig, + SubagentFeatureState, + SubagentPolicySource, + SubagentPolicyValidationContext, +} from './policy'; + +export interface SubagentModelPolicySnapshot { + readonly policy: CanonicalSubagentModelPolicy; + readonly resourceVersion: string; +} + +export interface EffectiveSubagentModelPolicy { + readonly configuredPolicy: CanonicalSubagentModelPolicy; + readonly effectivePolicy: CanonicalSubagentModelPolicy; + readonly policySource: SubagentPolicySource; + readonly feature: SubagentFeatureState; +} + +export interface PreparedSubagentPolicyMutation { + readonly policy: CanonicalSubagentModelPolicy; + readonly section: LegacySecondaryModelConfig | undefined; +} + +export interface SubagentCallerBinding { + readonly modelAlias: string; + readonly thinkingLevel?: string; +} + +export interface ISubagentModelPolicyService { + readonly _serviceBrand: undefined; + + get(): SubagentModelPolicySnapshot; + getEffective(): EffectiveSubagentModelPolicy; + set(policy: unknown, expectedVersion?: string): Promise; + clear(expectedVersion?: string): Promise; + prepareLegacyMutation( + input: unknown, + context?: SubagentPolicyValidationContext, + ): PreparedSubagentPolicyMutation; + resolveRevision(caller: SubagentCallerBinding): string; +} + +export const ISubagentModelPolicyService: ServiceIdentifier = + createDecorator('subagentModelPolicyService'); diff --git a/packages/agent-core-v2/src/session/subagent/subagentModelPolicyService.ts b/packages/agent-core-v2/src/session/subagent/subagentModelPolicyService.ts new file mode 100644 index 000000000..962186b86 --- /dev/null +++ b/packages/agent-core-v2/src/session/subagent/subagentModelPolicyService.ts @@ -0,0 +1,190 @@ +import { LifecycleScope } from '#/app/scopes'; +import { ScopeActivation, registerScopedService } from '#/_base/di/scope'; +import { Error2, ErrorCodes } from '#/errors'; +import { IConfigService } from '#/app/config/config'; +import { IFlagService } from '#/app/flag/flag'; +import { THINKING_SECTION } from '#/app/kosongConfig/configSection'; +import { IModelCatalog } from '#/kosong/model/catalog'; +import { declaredDefaultEffortForModel, type ThinkingConfig } from '#/kosong/model/thinking'; + +import { SECONDARY_MODEL_FLAG_ID } from './flag'; +import { + INHERIT_SUBAGENT_MODEL_POLICY, + type LegacySecondaryModelConfig, + LegacySecondaryModelConfigSchema, + normalizeLegacySecondaryModel, + normalizeLegacySecondaryModelOrInherit, + parseCanonicalSubagentModelPolicy, + routingEnvironmentRevision, + SECONDARY_MODEL_SECTION, + type SubagentFeatureState, + type SubagentPolicyValidationContext, + subagentPolicyResourceVersion, + toPersistedSecondaryModel, + validateSubagentModelPolicy, +} from './policy'; +import { + type EffectiveSubagentModelPolicy, + ISubagentModelPolicyService, + type PreparedSubagentPolicyMutation, + type SubagentCallerBinding, + type SubagentModelPolicySnapshot, +} from './subagentModelPolicy'; + +export class SubagentModelPolicyService implements ISubagentModelPolicyService { + declare readonly _serviceBrand: undefined; + + private commitChain: Promise = Promise.resolve(); + + constructor( + @IConfigService private readonly config: IConfigService, + @IFlagService private readonly flags: IFlagService, + @IModelCatalog private readonly modelCatalog: IModelCatalog, + ) {} + + get(): SubagentModelPolicySnapshot { + const persisted = this.persisted(); + return { + policy: normalizeLegacySecondaryModelOrInherit(persisted), + resourceVersion: subagentPolicyResourceVersion(persisted), + }; + } + + getEffective(): EffectiveSubagentModelPolicy { + const section = this.config.get(SECONDARY_MODEL_SECTION); + const configuredPolicy = normalizeLegacySecondaryModelOrInherit(section); + const feature = this.feature(); + return { + configuredPolicy, + effectivePolicy: feature.enabled ? configuredPolicy : INHERIT_SUBAGENT_MODEL_POLICY, + policySource: + feature.enabled && section !== undefined && configuredPolicy.mode !== 'inherit' + ? 'config' + : 'default', + feature, + }; + } + + async set(input: unknown, expectedVersion?: string): Promise { + const policy = parseCanonicalSubagentModelPolicy(input); + validateSubagentModelPolicy(policy, this.liveContext()); + await this.commit(toPersistedSecondaryModel(policy), expectedVersion); + return this.get(); + } + + async clear(expectedVersion?: string): Promise { + await this.commit(undefined, expectedVersion); + return this.get(); + } + + prepareLegacyMutation( + input: unknown, + context: SubagentPolicyValidationContext = this.liveContext(), + ): PreparedSubagentPolicyMutation { + if (input === null || input === undefined) { + return { policy: INHERIT_SUBAGENT_MODEL_POLICY, section: undefined }; + } + const parsed = LegacySecondaryModelConfigSchema.safeParse(input); + if (!parsed.success) { + const issue = parsed.error.issues[0]; + throw new Error2( + ErrorCodes.CONFIG_INVALID, + `Invalid [secondary_model] section: ${issue?.path.join('.') ?? ''} ${issue?.message ?? 'malformed'}`.trim(), + { details: { section: SECONDARY_MODEL_SECTION } }, + ); + } + const policy = normalizeLegacySecondaryModel(parsed.data); + validateSubagentModelPolicy(policy, context); + return { policy, section: toPersistedSecondaryModel(policy) }; + } + + resolveRevision(caller: SubagentCallerBinding): string { + const effective = this.getEffective(); + const boundModel = + effective.effectivePolicy.mode === 'inherit' + ? caller.modelAlias + : effective.effectivePolicy.defaultModel; + return routingEnvironmentRevision({ + effectivePolicy: effective.effectivePolicy, + policySource: effective.policySource, + feature: effective.feature, + callerModel: caller.modelAlias, + callerThinking: caller.thinkingLevel, + thinkingEnabled: + this.config.get(THINKING_SECTION)?.enabled !== false, + boundModelDefaultEffort: this.declaredDefaultEffort(boundModel), + }); + } + + private persisted(): LegacySecondaryModelConfig | undefined { + return this.config.inspect(SECONDARY_MODEL_SECTION) + .userValue; + } + + private feature(): SubagentFeatureState { + const state = this.flags.explain(SECONDARY_MODEL_FLAG_ID); + return { + enabled: state?.enabled ?? this.flags.enabled(SECONDARY_MODEL_FLAG_ID), + source: state?.source ?? 'default', + }; + } + + private liveContext(): SubagentPolicyValidationContext { + return { + resolveModel: (alias) => { + const model = this.modelCatalog.get(alias); + return { + id: model.id, + defaultEffort: model.defaultEffort, + supportEfforts: model.supportEfforts, + }; + }, + }; + } + + private declaredDefaultEffort(alias: string): string | undefined { + try { + return declaredDefaultEffortForModel(this.modelCatalog.get(alias)); + } catch { + return undefined; + } + } + + private commit( + section: LegacySecondaryModelConfig | undefined, + expectedVersion: string | undefined, + ): Promise { + const run = this.commitChain.then( + () => this.commitNow(section, expectedVersion), + () => this.commitNow(section, expectedVersion), + ); + this.commitChain = run.catch(() => undefined); + return run; + } + + private async commitNow( + section: LegacySecondaryModelConfig | undefined, + expectedVersion: string | undefined, + ): Promise { + await this.config.ready; + if (expectedVersion !== undefined) { + const currentVersion = subagentPolicyResourceVersion(this.persisted()); + if (currentVersion !== expectedVersion) { + throw new Error2( + ErrorCodes.CONFIG_VERSION_CONFLICT, + 'The subagent model policy changed since it was read; reload and retry.', + { details: { section: SECONDARY_MODEL_SECTION, expectedVersion, currentVersion } }, + ); + } + } + await this.config.replace(SECONDARY_MODEL_SECTION, section); + } +} + +registerScopedService( + LifecycleScope.App, + ISubagentModelPolicyService, + SubagentModelPolicyService, + ScopeActivation.OnScopeCreated, + 'subagent', +); 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 1548f3c84..c380db601 100644 --- a/packages/agent-core-v2/test/app/config/config.test.ts +++ b/packages/agent-core-v2/test/app/config/config.test.ts @@ -89,6 +89,7 @@ import { type SubagentConfig, wrapSubagentModelError, } from '#/session/subagent/configSection'; +import { prospectiveModelView, validateSubagentModelPolicy } from '#/session/subagent/policy'; import { SECONDARY_MODEL_FLAG_ID } from '#/session/subagent/flag'; import { DEFAULT_DYNAMIC_WORKFLOW_TIMEOUT_MS, @@ -2702,6 +2703,85 @@ describe('ConfigService replaceSections', () => { disposables.dispose(); }); + it('previewReplaceSections returns the prospective effective config with zero side effects', async () => { + const { config, disposables, store, storage } = await createSectionsConfig(); + const setSpy = vi.spyOn(store, 'set'); + const events: string[] = []; + disposables.add(config.onDidChangeConfiguration((e) => events.push(e.domain))); + const diskBefore = new TextDecoder().decode(await storage.read('', 'config.toml')); + + const preview = config.previewReplaceSections({ + [MODELS_SECTION]: { 'acme/m2': { provider: 'acme', model: 'm2', maxContextSize: 2000 } }, + [DEFAULT_MODEL_SECTION]: null, + }); + + expect(Object.keys(preview[MODELS_SECTION] as Record)).toEqual(['acme/m2']); + expect(preview[DEFAULT_MODEL_SECTION]).toBeUndefined(); + expect(preview[PROVIDERS_SECTION]).toEqual({ acme: { type: 'openai', apiKey: 'sk-acme' } }); + expect(preview[THINKING_SECTION]).toEqual({ enabled: true }); + expect(setSpy).not.toHaveBeenCalled(); + expect(events).toEqual([]); + expect(config.get(DEFAULT_MODEL_SECTION)).toBe('acme/m1'); + expect(Object.keys(config.get>(MODELS_SECTION))).toEqual(['acme/m1']); + expect(new TextDecoder().decode(await storage.read('', 'config.toml'))).toBe(diskBefore); + + await config.replaceSections({ + [MODELS_SECTION]: { 'acme/m2': { provider: 'acme', model: 'm2', maxContextSize: 2000 } }, + [DEFAULT_MODEL_SECTION]: null, + }); + expect(config.get(MODELS_SECTION)).toEqual(preview[MODELS_SECTION]); + expect(config.get(DEFAULT_MODEL_SECTION)).toBeUndefined(); + + expect(() => config.previewReplaceSections({ [MODELS_SECTION]: { broken: 'x' } })).toThrow(); + expect(setSpy).toHaveBeenCalledTimes(1); + + disposables.dispose(); + }); + + it('a prospective model view from previewReplaceSections validates removals and swaps before any write', async () => { + const seed = [ + '[providers.acme]', + 'type = "openai"', + 'api_key = "sk-acme"', + '', + '[models."acme/luna"]', + 'provider = "acme"', + 'model = "luna"', + 'max_context_size = 1000', + '', + ].join('\n'); + const { config, disposables, store } = await createSectionsConfig(seed); + const setSpy = vi.spyOn(store, 'set'); + const luna = { mode: 'default', defaultModel: 'acme/luna' } as const; + const sol = { mode: 'default', defaultModel: 'acme/sol' } as const; + const solRecord = { provider: 'acme', model: 'sol', maxContextSize: 1000 }; + + const current = config.previewReplaceSections({}); + expect(() => + validateSubagentModelPolicy(luna, prospectiveModelView(current[PROVIDERS_SECTION], current[MODELS_SECTION])), + ).not.toThrow(); + + const removed = config.previewReplaceSections({ [MODELS_SECTION]: {} }); + expect(() => + validateSubagentModelPolicy(luna, prospectiveModelView(removed[PROVIDERS_SECTION], removed[MODELS_SECTION])), + ).toThrow(/acme\/luna/); + + const swapped = config.previewReplaceSections({ [MODELS_SECTION]: { 'acme/sol': solRecord } }); + const swappedView = prospectiveModelView(swapped[PROVIDERS_SECTION], swapped[MODELS_SECTION]); + expect(() => validateSubagentModelPolicy(sol, swappedView)).not.toThrow(); + expect(() => validateSubagentModelPolicy(luna, swappedView)).toThrow(); + expect(setSpy).not.toHaveBeenCalled(); + + await config.replaceSections({ + [MODELS_SECTION]: { 'acme/sol': solRecord }, + [SECONDARY_MODEL_SECTION]: { defaultModel: 'acme/sol' }, + }); + expect(setSpy).toHaveBeenCalledTimes(1); + expect(config.get(SECONDARY_MODEL_SECTION)).toEqual({ defaultModel: 'acme/sol' }); + + disposables.dispose(); + }); + it('fires change events only after all domains have taken effect', async () => { const { config, disposables } = await createSectionsConfig(); const domains: string[] = []; diff --git a/packages/agent-core-v2/test/app/kosongConfig/discovery.test.ts b/packages/agent-core-v2/test/app/kosongConfig/discovery.test.ts index ee13d3ada..5ab00ed13 100644 --- a/packages/agent-core-v2/test/app/kosongConfig/discovery.test.ts +++ b/packages/agent-core-v2/test/app/kosongConfig/discovery.test.ts @@ -6,6 +6,15 @@ import { ILogService, type LogPayload } from '#/_base/log/log'; import { IAgentIdentity } from '#/app/agentIdentity/agentIdentity'; import { IBootstrapService } from '#/app/bootstrap/bootstrap'; import { IConfigService } from '#/app/config/config'; +import { + LegacySecondaryModelConfigSchema, + normalizeLegacySecondaryModel, + toPersistedSecondaryModel, +} from '#/session/subagent/policy'; +import { + ISubagentModelPolicyService, + type PreparedSubagentPolicyMutation, +} from '#/session/subagent/subagentModelPolicy'; import { ConfigRegistry } from '#/app/config/configService'; import { IEventService } from '#/app/event/event'; import { IProviderDiscoveryService } from '#/app/kosongConfig/discovery'; @@ -60,6 +69,29 @@ function stubLogService(): ILogService { } satisfies ILogService; } +function stubSubagentModelPolicy(): ISubagentModelPolicyService { + const prepare = (input: unknown): PreparedSubagentPolicyMutation => { + const policy = normalizeLegacySecondaryModel( + input === null || input === undefined ? undefined : LegacySecondaryModelConfigSchema.parse(input), + ); + return { policy, section: toPersistedSecondaryModel(policy) }; + }; + return { + _serviceBrand: undefined, + get: () => ({ policy: { mode: 'inherit' }, resourceVersion: 'stub' }), + getEffective: () => ({ + configuredPolicy: { mode: 'inherit' }, + effectivePolicy: { mode: 'inherit' }, + policySource: 'default', + feature: { enabled: false, source: 'default' }, + }), + set: () => Promise.reject(new Error('not stubbed')), + clear: () => Promise.reject(new Error('not stubbed')), + prepareLegacyMutation: prepare, + resolveRevision: () => 'stub', + }; +} + async function createHost( sections: Record = {}, ): Promise<{ @@ -75,6 +107,7 @@ async function createHost( const host = createScopedTestHost([ [IConfigService, config], [IEventService, events], + [ISubagentModelPolicyService, stubSubagentModelPolicy()], [ILogService, stubLogService()], [ IBootstrapService, diff --git a/packages/agent-core-v2/test/kosong/stubs.ts b/packages/agent-core-v2/test/kosong/stubs.ts index b5530f052..05142547d 100644 --- a/packages/agent-core-v2/test/kosong/stubs.ts +++ b/packages/agent-core-v2/test/kosong/stubs.ts @@ -64,6 +64,18 @@ export class StubConfigService implements IConfigService { return Promise.resolve(); } + previewReplaceSections(sections: Readonly>): ResolvedConfig { + const next = Object.fromEntries(this._values) as ResolvedConfig; + for (const [domain, value] of Object.entries(sections)) { + if (value === undefined || value === null) { + delete next[domain]; + } else { + next[domain] = value; + } + } + return next; + } + replaceSections(sections: Readonly>): Promise { for (const [domain, value] of Object.entries(sections)) { const previousValue = this._values.get(domain); diff --git a/packages/agent-core-v2/test/session/subagent/policy.test.ts b/packages/agent-core-v2/test/session/subagent/policy.test.ts new file mode 100644 index 000000000..44589d6e6 --- /dev/null +++ b/packages/agent-core-v2/test/session/subagent/policy.test.ts @@ -0,0 +1,358 @@ +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { dirname, join, relative } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { describe, expect, it } from 'vitest'; + +import { ErrorCodes, isError2 } from '#/errors'; +import { + type CanonicalSubagentModelPolicy, + INHERIT_SUBAGENT_MODEL_POLICY, + type LegacySecondaryModelConfig, + normalizeLegacySecondaryModel, + normalizeLegacySecondaryModelOrInherit, + parseCanonicalSubagentModelPolicy, + prospectiveModelView, + ROUTE_DECISION_FINGERPRINT_PREFIX, + ROUTING_ENVIRONMENT_REVISION_PREFIX, + routeDecisionFingerprint, + routingEnvironmentRevision, + SECONDARY_MODEL_DEFAULT_MODEL_REQUIRED_MESSAGE, + SECONDARY_MODEL_FORCE_EXCLUDES_MODELS_MESSAGE, + SECONDARY_MODEL_FORCE_REQUIRES_DEFAULT_MESSAGE, + SUBAGENT_POLICY_RESOURCE_VERSION_PREFIX, + subagentPolicyResourceVersion, + toPersistedSecondaryModel, + validateSubagentModelPolicy, +} from '#/session/subagent/policy'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const SRC_ROOT = join(__dirname, '..', '..', '..', 'src'); + +function codeOf(fn: () => unknown): string | undefined { + try { + fn(); + return undefined; + } catch (error) { + return isError2(error) ? error.code : 'not-error2'; + } +} + +function messageOf(fn: () => unknown): string { + try { + fn(); + return ''; + } catch (error) { + return error instanceof Error ? error.message : String(error); + } +} + +describe('normalizeLegacySecondaryModel', () => { + const D = 'acme/default'; + const M = 'acme/legacy'; + const POOL = { 'acme/default': 'fast', 'acme/other': '' }; + + const table: Array<{ + legacy: LegacySecondaryModelConfig; + expected: CanonicalSubagentModelPolicy | 'invalid'; + }> = [ + { legacy: {}, expected: INHERIT_SUBAGENT_MODEL_POLICY }, + { legacy: { model: M }, expected: { mode: 'default', defaultModel: M } }, + { legacy: { defaultModel: D }, expected: { mode: 'default', defaultModel: D } }, + { legacy: { defaultModel: D, model: M }, expected: { mode: 'default', defaultModel: D } }, + { legacy: { models: POOL }, expected: 'invalid' }, + { legacy: { models: POOL, model: M }, expected: 'invalid' }, + { legacy: { models: POOL, defaultModel: D }, expected: { mode: 'pool', defaultModel: D, models: POOL } }, + { + legacy: { models: POOL, defaultModel: D, model: M }, + expected: { mode: 'pool', defaultModel: D, models: POOL }, + }, + { legacy: { force: true }, expected: 'invalid' }, + { legacy: { force: true, model: M }, expected: { mode: 'force', defaultModel: M } }, + { legacy: { force: true, defaultModel: D }, expected: { mode: 'force', defaultModel: D } }, + { legacy: { force: true, defaultModel: D, model: M }, expected: { mode: 'force', defaultModel: D } }, + { legacy: { force: true, models: POOL }, expected: 'invalid' }, + { legacy: { force: true, models: POOL, model: M }, expected: 'invalid' }, + { legacy: { force: true, models: POOL, defaultModel: D }, expected: 'invalid' }, + { legacy: { force: true, models: POOL, defaultModel: D, model: M }, expected: 'invalid' }, + ]; + + it.each(table)('normalizes $legacy', ({ legacy, expected }) => { + if (expected === 'invalid') { + expect(codeOf(() => normalizeLegacySecondaryModel(legacy))).toBe(ErrorCodes.CONFIG_INVALID); + expect(normalizeLegacySecondaryModelOrInherit(legacy)).toEqual(INHERIT_SUBAGENT_MODEL_POLICY); + return; + } + const policy = normalizeLegacySecondaryModel(legacy); + expect(policy).toMatchObject(expected); + expect(policy).not.toHaveProperty('model'); + expect(policy).not.toHaveProperty('force'); + }); + + it('keeps the legacy error messages for the invalid combinations', () => { + expect(messageOf(() => normalizeLegacySecondaryModel({ force: true }))).toBe( + SECONDARY_MODEL_FORCE_REQUIRES_DEFAULT_MESSAGE, + ); + expect(messageOf(() => normalizeLegacySecondaryModel({ force: true, defaultModel: D, models: POOL }))).toBe( + SECONDARY_MODEL_FORCE_EXCLUDES_MODELS_MESSAGE, + ); + expect(messageOf(() => normalizeLegacySecondaryModel({ models: POOL }))).toBe( + SECONDARY_MODEL_DEFAULT_MODEL_REQUIRED_MESSAGE, + ); + }); + + it('treats an absent section as inherit and never carries legacy fields', () => { + expect(normalizeLegacySecondaryModel(undefined)).toEqual({ mode: 'inherit' }); + const withExtras = normalizeLegacySecondaryModel({ + defaultModel: D, + defaultEffort: 'low', + maxContextSize: 1000, + displayName: 'x', + }); + expect(withExtras).toEqual({ mode: 'default', defaultModel: D, defaultEffort: 'low' }); + }); + + it('round-trips canonical policies through the persisted form', () => { + const policies: CanonicalSubagentModelPolicy[] = [ + { mode: 'inherit' }, + { mode: 'default', defaultModel: D, defaultEffort: 'high' }, + { mode: 'pool', defaultModel: D, models: POOL }, + { mode: 'force', defaultModel: D }, + ]; + for (const policy of policies) { + const persisted = toPersistedSecondaryModel(policy); + expect(normalizeLegacySecondaryModel(persisted)).toEqual( + JSON.parse(JSON.stringify(policy)), + ); + } + expect(toPersistedSecondaryModel({ mode: 'inherit' })).toBeUndefined(); + expect(toPersistedSecondaryModel({ mode: 'force', defaultModel: D })).toMatchObject({ + force: true, + }); + }); + + it('parses canonical input strictly', () => { + expect(parseCanonicalSubagentModelPolicy({ mode: 'inherit' })).toEqual({ mode: 'inherit' }); + expect(codeOf(() => parseCanonicalSubagentModelPolicy({ mode: 'force' }))).toBe( + ErrorCodes.CONFIG_INVALID, + ); + expect(codeOf(() => parseCanonicalSubagentModelPolicy({ mode: 'default', defaultModel: D, force: true }))).toBe( + ErrorCodes.CONFIG_INVALID, + ); + expect(codeOf(() => parseCanonicalSubagentModelPolicy({ mode: 'later' }))).toBe( + ErrorCodes.CONFIG_INVALID, + ); + }); +}); + +describe('validateSubagentModelPolicy', () => { + const known = (ids: Record) => ({ + resolveModel: (alias: string) => + Object.hasOwn(ids, alias) ? { id: alias, supportEfforts: ids[alias]?.supportEfforts } : undefined, + }); + + it('accepts inherit without a catalog', () => { + expect(() => + validateSubagentModelPolicy({ mode: 'inherit' }, { resolveModel: () => undefined }), + ).not.toThrow(); + }); + + it('rejects unknown models, a default outside the pool, the reserved primary key, and unsupported efforts', () => { + const ctx = known({ 'acme/a': { supportEfforts: ['low', 'high'] }, 'acme/b': {} }); + expect(codeOf(() => validateSubagentModelPolicy({ mode: 'default', defaultModel: 'acme/zzz' }, ctx))).toBe( + ErrorCodes.CONFIG_INVALID, + ); + expect( + codeOf(() => + validateSubagentModelPolicy( + { mode: 'pool', defaultModel: 'acme/b', models: { 'acme/a': '' } }, + ctx, + ), + ), + ).toBe(ErrorCodes.CONFIG_INVALID); + expect( + codeOf(() => + validateSubagentModelPolicy( + { mode: 'pool', defaultModel: 'acme/a', models: { 'acme/a': '', primary: '' } }, + ctx, + ), + ), + ).toBe(ErrorCodes.CONFIG_INVALID); + expect( + codeOf(() => + validateSubagentModelPolicy({ mode: 'force', defaultModel: 'acme/a', defaultEffort: 'max' }, ctx), + ), + ).toBe(ErrorCodes.CONFIG_INVALID); + expect(() => + validateSubagentModelPolicy({ mode: 'force', defaultModel: 'acme/a', defaultEffort: 'high' }, ctx), + ).not.toThrow(); + expect(() => + validateSubagentModelPolicy({ mode: 'default', defaultModel: 'acme/b', defaultEffort: 'anything' }, ctx), + ).not.toThrow(); + }); + + it('resolves models from a prospective config view including aliases and provider presence', () => { + const view = prospectiveModelView( + { acme: { type: 'openai' } }, + { + 'acme/a': { provider: 'acme', model: 'a', aliases: ['fast'], supportEfforts: ['low'] }, + 'gone/b': { provider: 'gone', model: 'b' }, + }, + ); + expect(view.resolveModel('acme/a')).toEqual({ id: 'acme/a', defaultEffort: undefined, supportEfforts: ['low'] }); + expect(view.resolveModel('fast')?.id).toBe('fast'); + expect(view.resolveModel('gone/b')).toBeUndefined(); + expect(view.resolveModel('missing')).toBeUndefined(); + expect(prospectiveModelView(undefined, undefined).resolveModel('x')).toBeUndefined(); + }); +}); + +describe('subagentPolicyResourceVersion', () => { + it('is strong, stable across key order and equivalent legacy spellings, and exists for the absent section', () => { + const a = subagentPolicyResourceVersion({ defaultModel: 'acme/a', models: { x: '', y: '' } }); + const b = subagentPolicyResourceVersion({ models: { y: '', x: '' }, defaultModel: 'acme/a' }); + expect(a).toBe(b); + expect(a.startsWith(SUBAGENT_POLICY_RESOURCE_VERSION_PREFIX)).toBe(true); + expect(a.startsWith('W/')).toBe(false); + expect(subagentPolicyResourceVersion({ model: 'acme/a' })).toBe( + subagentPolicyResourceVersion({ defaultModel: 'acme/a' }), + ); + expect(subagentPolicyResourceVersion({ defaultModel: 'acme/a', force: false })).toBe( + subagentPolicyResourceVersion({ defaultModel: 'acme/a' }), + ); + const absent = subagentPolicyResourceVersion(undefined); + expect(absent.startsWith(SUBAGENT_POLICY_RESOURCE_VERSION_PREFIX)).toBe(true); + expect(absent).not.toBe(a); + expect(subagentPolicyResourceVersion({ defaultModel: 'acme/b' })).not.toBe(a); + expect(subagentPolicyResourceVersion({ force: true })).not.toBe(absent); + }); +}); + +describe('routing revisions', () => { + const base = { + effectivePolicy: { mode: 'inherit' } as CanonicalSubagentModelPolicy, + policySource: 'default' as const, + feature: { enabled: false, source: 'default' as const }, + callerModel: 'acme/sol', + callerThinking: 'high', + thinkingEnabled: true, + boundModelDefaultEffort: 'high', + }; + + it('changes only with ambient inputs and ignores key order', () => { + const rev = routingEnvironmentRevision(base); + expect(rev.startsWith(ROUTING_ENVIRONMENT_REVISION_PREFIX)).toBe(true); + expect(routingEnvironmentRevision({ ...base })).toBe(rev); + expect(routingEnvironmentRevision({ ...base, callerModel: 'acme/luna' })).not.toBe(rev); + expect( + routingEnvironmentRevision({ ...base, feature: { enabled: true, source: 'env' } }), + ).not.toBe(rev); + const pool = { + ...base, + effectivePolicy: { + mode: 'pool', + defaultModel: 'acme/a', + models: { 'acme/a': '', 'acme/b': '' }, + } as CanonicalSubagentModelPolicy, + }; + const poolReordered = { + ...pool, + effectivePolicy: { + mode: 'pool', + defaultModel: 'acme/a', + models: { 'acme/b': '', 'acme/a': '' }, + } as CanonicalSubagentModelPolicy, + }; + expect(routingEnvironmentRevision(pool)).toBe(routingEnvironmentRevision(poolReordered)); + }); + + it('keeps request intent out of the environment revision and inside the decision fingerprint', () => { + const rev = routingEnvironmentRevision(base); + const spawnA = routeDecisionFingerprint({ + routingEnvironmentRevision: rev, + operation: 'spawn', + model: 'acme/a', + }); + const spawnB = routeDecisionFingerprint({ + routingEnvironmentRevision: rev, + operation: 'spawn', + model: 'acme/b', + }); + expect(spawnA.startsWith(ROUTE_DECISION_FINGERPRINT_PREFIX)).toBe(true); + expect(spawnA).not.toBe(spawnB); + expect( + routeDecisionFingerprint({ routingEnvironmentRevision: rev, operation: 'spawn', model: 'acme/a' }), + ).toBe(spawnA); + expect( + routeDecisionFingerprint({ routingEnvironmentRevision: rev, operation: 'fork', model: 'acme/a' }), + ).not.toBe(spawnA); + }); +}); + +describe('legacy secondary-model import boundary', () => { + const LEGACY_SYMBOLS = + /\b(LegacySecondaryModelConfig|LegacySecondaryModelConfigSchema|normalizeLegacySecondaryModel|normalizeLegacySecondaryModelOrInherit|toPersistedSecondaryModel|SecondaryModelConfig|SecondaryModelConfigSchema)\b/; + const ALLOWED = new Set([ + 'index.ts', + 'session/subagent/policy.ts', + 'session/subagent/configSection.ts', + 'session/subagent/subagentModelPolicy.ts', + 'session/subagent/subagentModelPolicyService.ts', + ]); + const WRITE_RE = + /(\.(replace|set)\(\s*SECONDARY_MODEL_SECTION\b|\[SECONDARY_MODEL_SECTION\]\s*[:=])/; + const WRITERS = new Set([ + 'session/subagent/subagentModelPolicyService.ts', + 'app/kosongConfig/discoveryService.ts', + ]); + + function walk(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir)) { + const abs = join(dir, entry); + if (statSync(abs).isDirectory()) out.push(...walk(abs)); + else if (abs.endsWith('.ts')) out.push(abs); + } + return out; + } + + it('only the legacy adapter, the policy service and the root index touch legacy secondary-model symbols', () => { + const offenders: string[] = []; + for (const file of walk(SRC_ROOT)) { + const rel = relative(SRC_ROOT, file); + if (ALLOWED.has(rel)) continue; + const source = readFileSync(file, 'utf8'); + if (LEGACY_SYMBOLS.test(source)) offenders.push(rel); + } + expect(offenders).toEqual([]); + }); + + it('the secondary-model section is written only by the policy service or a prepared mutation', () => { + const offenders: string[] = []; + for (const file of walk(SRC_ROOT)) { + const rel = relative(SRC_ROOT, file); + const source = readFileSync(file, 'utf8'); + if (!WRITE_RE.test(source)) continue; + if (WRITERS.has(rel)) { + if (rel !== 'session/subagent/subagentModelPolicyService.ts') { + expect(source, rel).toContain('prepareLegacyMutation('); + } + continue; + } + offenders.push(rel); + } + expect(offenders).toEqual([]); + }); + + it('boundary scans see a positive control', () => { + const policySource = readFileSync(join(SRC_ROOT, 'session/subagent/policy.ts'), 'utf8'); + expect(LEGACY_SYMBOLS.test(policySource)).toBe(true); + const serviceSource = readFileSync( + join(SRC_ROOT, 'session/subagent/subagentModelPolicyService.ts'), + 'utf8', + ); + expect(WRITE_RE.test(serviceSource)).toBe(true); + const discoverySource = readFileSync(join(SRC_ROOT, 'app/kosongConfig/discoveryService.ts'), 'utf8'); + expect(WRITE_RE.test(discoverySource)).toBe(true); + }); +}); diff --git a/packages/agent-core-v2/test/session/subagent/subagentModelPolicyService.test.ts b/packages/agent-core-v2/test/session/subagent/subagentModelPolicyService.test.ts new file mode 100644 index 000000000..7e23d8ac0 --- /dev/null +++ b/packages/agent-core-v2/test/session/subagent/subagentModelPolicyService.test.ts @@ -0,0 +1,216 @@ +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 { type ExperimentalFeatureState, IFlagService } from '#/app/flag/flag'; +import { THINKING_SECTION } from '#/app/kosongConfig/configSection'; +import { ErrorCodes, Error2, isError2 } from '#/errors'; +import { IModelCatalog, type Model } from '#/kosong/model/catalog'; +import { SECONDARY_MODEL_FLAG_ID } from '#/session/subagent/flag'; +import { SECONDARY_MODEL_SECTION } from '#/session/subagent/policy'; +import { + ISubagentModelPolicyService, + type SubagentModelPolicySnapshot, +} from '#/session/subagent/subagentModelPolicy'; +import { SubagentModelPolicyService } from '#/session/subagent/subagentModelPolicyService'; + +import { StubConfigService } from '../../kosong/stubs'; +import { stubFlag } from '../../app/flag/stubs'; + +describe('SubagentModelPolicyService', () => { + let disposables: DisposableStore; + let ix: TestInstantiationService; + let config: StubConfigService; + let models: Map>; + + beforeEach(() => { + disposables = new DisposableStore(); + ix = disposables.add(new TestInstantiationService()); + models = new Map([ + ['acme/sol', { id: 'acme/sol', supportEfforts: ['low', 'high'], defaultEffort: 'high' }], + ['acme/luna', { id: 'acme/luna', supportEfforts: ['low', 'high'] }], + ]); + }); + afterEach(() => disposables.dispose()); + + function setup( + sections: Record, + feature: { enabled: boolean; source?: ExperimentalFeatureState['source'] } = { enabled: true }, + ): ISubagentModelPolicyService { + config = new StubConfigService(sections); + ix.stub(IConfigService, config); + const flags = stubFlag((id) => feature.enabled && id === SECONDARY_MODEL_FLAG_ID); + ix.stub(IFlagService, { + ...flags, + explain: (id: string) => + id === SECONDARY_MODEL_FLAG_ID + ? ({ + id, + enabled: feature.enabled, + source: feature.source ?? 'config', + externallyControlled: feature.source === 'env', + overridden: false, + defaultEnabled: false, + title: '', + description: '', + surface: 'core', + env: '', + } as ExperimentalFeatureState) + : undefined, + }); + ix.stub(IModelCatalog, { + _serviceBrand: undefined, + get: (id: string) => { + const model = models.get(id); + if (model === undefined) { + throw new Error2(ErrorCodes.CONFIG_INVALID, `Model "${id}" is not configured in config.toml.`, { + details: { model: id }, + }); + } + return model as Model; + }, + } as unknown as IModelCatalog); + ix.set(ISubagentModelPolicyService, new SyncDescriptor(SubagentModelPolicyService)); + return ix.get(ISubagentModelPolicyService); + } + + async function codeOf(fn: () => Promise | unknown): Promise { + try { + await fn(); + return undefined; + } catch (error) { + return isError2(error) ? error.code : 'not-error2'; + } + } + + it('reads the persisted policy with a resource version and reports inherit for an absent section', () => { + const service = setup({}); + const snapshot = service.get(); + expect(snapshot.policy).toEqual({ mode: 'inherit' }); + expect(snapshot.resourceVersion).toMatch(/^subagent-policy-v1:/); + }); + + it('set validates against the live catalog and persists the canonical section', async () => { + const service = setup({}); + await expect( + service.set({ mode: 'default', defaultModel: 'acme/nope' }), + ).rejects.toMatchObject({ code: ErrorCodes.CONFIG_INVALID }); + expect(config.get(SECONDARY_MODEL_SECTION)).toBeUndefined(); + + const snapshot = await service.set({ mode: 'force', defaultModel: 'acme/sol', defaultEffort: 'low' }); + expect(snapshot.policy).toEqual({ mode: 'force', defaultModel: 'acme/sol', defaultEffort: 'low' }); + expect(config.get(SECONDARY_MODEL_SECTION)).toEqual({ + defaultModel: 'acme/sol', + force: true, + defaultEffort: 'low', + }); + + await service.set({ mode: 'default', defaultModel: 'acme/luna' }); + expect(config.get(SECONDARY_MODEL_SECTION)).toEqual({ + defaultModel: 'acme/luna', + defaultEffort: undefined, + }); + }); + + it('clear removes the section and expectedVersion guards both set and clear', async () => { + const service = setup({ [SECONDARY_MODEL_SECTION]: { defaultModel: 'acme/sol' } }); + const before = service.get().resourceVersion; + expect(await codeOf(() => service.set({ mode: 'inherit' }, 'subagent-policy-v1:stale'))).toBe( + ErrorCodes.CONFIG_VERSION_CONFLICT, + ); + expect(config.get(SECONDARY_MODEL_SECTION)).toEqual({ defaultModel: 'acme/sol' }); + const cleared = await service.clear(before); + expect(cleared.policy).toEqual({ mode: 'inherit' }); + expect(config.get(SECONDARY_MODEL_SECTION)).toBeUndefined(); + expect(cleared.resourceVersion).not.toBe(before); + expect(await codeOf(() => service.clear(before))).toBe(ErrorCodes.CONFIG_VERSION_CONFLICT); + }); + + it('serializes concurrent commits so a stale expectedVersion cannot slip past the version check', async () => { + const service = setup({ [SECONDARY_MODEL_SECTION]: { defaultModel: 'acme/sol' } }); + const replace = config.replace.bind(config); + config.replace = async (domain, value) => { + await new Promise((resolve) => setTimeout(resolve, 0)); + await replace(domain, value); + }; + const version = service.get().resourceVersion; + const [first, second] = await Promise.allSettled([ + service.set({ mode: 'default', defaultModel: 'acme/luna' }, version), + service.set({ mode: 'force', defaultModel: 'acme/sol' }, version), + ]); + expect(first.status).toBe('fulfilled'); + expect(second.status).toBe('rejected'); + expect((second as PromiseRejectedResult).reason).toMatchObject({ + code: ErrorCodes.CONFIG_VERSION_CONFLICT, + }); + expect(config.get(SECONDARY_MODEL_SECTION)).toEqual({ defaultModel: 'acme/luna', defaultEffort: undefined }); + const recovered = await service.set( + { mode: 'force', defaultModel: 'acme/sol' }, + (first as PromiseFulfilledResult).value.resourceVersion, + ); + expect(recovered.policy).toEqual({ mode: 'force', defaultModel: 'acme/sol' }); + expect(config.get(SECONDARY_MODEL_SECTION)).toEqual({ defaultModel: 'acme/sol', force: true, defaultEffort: undefined }); + }); + + it('getEffective reports inherit while the feature is disabled and keeps the configured policy', () => { + const service = setup( + { [SECONDARY_MODEL_SECTION]: { defaultModel: 'acme/sol', force: true } }, + { enabled: false, source: 'default' }, + ); + const effective = service.getEffective(); + expect(effective.configuredPolicy).toEqual({ mode: 'force', defaultModel: 'acme/sol' }); + expect(effective.effectivePolicy).toEqual({ mode: 'inherit' }); + expect(effective.policySource).toBe('default'); + expect(effective.feature).toEqual({ enabled: false, source: 'default' }); + }); + + it('prepareLegacyMutation validates against the supplied prospective context, not the live catalog', () => { + const service = setup({}); + const prospective = { + resolveModel: (alias: string) => (alias === 'acme/future' ? { id: alias } : undefined), + }; + const prepared = service.prepareLegacyMutation( + { defaultModel: 'acme/future', models: { 'acme/future': 'soon' } }, + prospective, + ); + expect(prepared.policy).toEqual({ + mode: 'pool', + defaultModel: 'acme/future', + models: { 'acme/future': 'soon' }, + defaultEffort: undefined, + }); + expect(prepared.section).toEqual({ + defaultModel: 'acme/future', + models: { 'acme/future': 'soon' }, + defaultEffort: undefined, + }); + expect(config.get(SECONDARY_MODEL_SECTION)).toBeUndefined(); + expect(() => service.prepareLegacyMutation({ defaultModel: 'acme/sol' }, prospective)).toThrow(); + expect(() => service.prepareLegacyMutation({ defaultModel: 'acme/sol' })).not.toThrow(); + expect(service.prepareLegacyMutation(null)).toEqual({ policy: { mode: 'inherit' }, section: undefined }); + expect(() => service.prepareLegacyMutation({ defaultModel: 42 })).toThrow(); + }); + + it('resolveRevision changes only with ambient routing inputs', async () => { + const service = setup({ [SECONDARY_MODEL_SECTION]: { defaultModel: 'acme/luna' } }); + const caller = { modelAlias: 'acme/sol', thinkingLevel: 'high' }; + const a = service.resolveRevision(caller); + expect(a).toMatch(/^route-env:v1:/); + expect(service.resolveRevision({ ...caller })).toBe(a); + expect(service.resolveRevision({ modelAlias: 'acme/luna', thinkingLevel: 'high' })).not.toBe(a); + await config.replace(THINKING_SECTION, { enabled: false }); + expect(service.resolveRevision(caller)).not.toBe(a); + }); + + it('resolveRevision ignores a configured policy while the feature is disabled', async () => { + const service = setup({}, { enabled: false, source: 'default' }); + const caller = { modelAlias: 'acme/sol', thinkingLevel: 'high' }; + const before = service.resolveRevision(caller); + const versionBefore = service.get().resourceVersion; + await config.replace(SECONDARY_MODEL_SECTION, { defaultModel: 'acme/luna', force: true }); + expect(service.resolveRevision(caller)).toBe(before); + expect(service.get().resourceVersion).not.toBe(versionBefore); + }); +}); 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 b31a61a90..fbf62ad5f 100644 --- a/packages/agent-core-v2/test/session/subagent/subagentModelsValidation.test.ts +++ b/packages/agent-core-v2/test/session/subagent/subagentModelsValidation.test.ts @@ -96,7 +96,7 @@ describe('SessionSubagentModelsValidationService', () => { 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', + '[secondary_model].default_model "provider/typo" could not be resolved', ); }); @@ -128,8 +128,9 @@ describe('SessionSubagentModelsValidationService', () => { 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', + '[secondary_model].default_model "provider/typo" could not be resolved', ); + expect((error as Error2).message).not.toContain('[secondary_model.models]'); }); it('constructs fine for a valid pool', () => { diff --git a/packages/agent-gateway/src/protocol/error-codes.ts b/packages/agent-gateway/src/protocol/error-codes.ts index f70d3caa0..60399cfa5 100644 --- a/packages/agent-gateway/src/protocol/error-codes.ts +++ b/packages/agent-gateway/src/protocol/error-codes.ts @@ -74,6 +74,8 @@ export const ErrorCode = { FS_PATH_ESCAPES_SESSION: 41304, FS_GREP_TIMEOUT: 41305, + CONFIG_VERSION_CONFLICT: 41201, + FS_WATCH_LIMIT_EXCEEDED: 42902, INTERNAL_ERROR: 50001, diff --git a/packages/agent-gateway/src/protocol/rest-config.ts b/packages/agent-gateway/src/protocol/rest-config.ts index 1f7056cc3..93cf0e4b4 100644 --- a/packages/agent-gateway/src/protocol/rest-config.ts +++ b/packages/agent-gateway/src/protocol/rest-config.ts @@ -34,6 +34,7 @@ export const configResponseSchema = z.object({ export type ConfigResponse = z.infer; const optionalModelAlias = z.string().min(1).optional(); +const droppedLegacyMetadata = z.unknown().optional(); export const legacySecondaryModelRequestSchema = z .object({ @@ -44,10 +45,68 @@ export const legacySecondaryModelRequestSchema = z defaultEffort: z.string().optional(), models: z.record(z.string(), z.string()).optional(), force: z.boolean().optional(), + max_context_size: droppedLegacyMetadata, + maxContextSize: droppedLegacyMetadata, + max_input_size: droppedLegacyMetadata, + maxInputSize: droppedLegacyMetadata, + max_output_size: droppedLegacyMetadata, + maxOutputSize: droppedLegacyMetadata, + capabilities: droppedLegacyMetadata, + display_name: droppedLegacyMetadata, + displayName: droppedLegacyMetadata, + reasoning_key: droppedLegacyMetadata, + reasoningKey: droppedLegacyMetadata, + adaptive_thinking: droppedLegacyMetadata, + adaptiveThinking: droppedLegacyMetadata, + support_efforts: droppedLegacyMetadata, + supportEfforts: droppedLegacyMetadata, + off_effort: droppedLegacyMetadata, + offEffort: droppedLegacyMetadata, }) .strict(); export type LegacySecondaryModelRequest = z.infer; +const policyModelAlias = z.string().min(1); +const policyEffort = z.string().min(1).optional(); + +export const subagentModelPolicyRequestSchema = z.discriminatedUnion('mode', [ + z.object({ mode: z.literal('inherit') }).strict(), + z.object({ mode: z.literal('default'), default_model: policyModelAlias, default_effort: policyEffort }).strict(), + z + .object({ + mode: z.literal('pool'), + default_model: policyModelAlias, + models: z.record(z.string(), z.string()), + default_effort: policyEffort, + }) + .strict(), + z.object({ mode: z.literal('force'), default_model: policyModelAlias, default_effort: policyEffort }).strict(), +]); +export type SubagentModelPolicyRequest = z.infer; + +export const subagentModelPolicyWireSchema = z.object({ + mode: z.enum(['inherit', 'default', 'pool', 'force']), + default_model: z.string().optional(), + models: z.record(z.string(), z.string()).optional(), + default_effort: z.string().optional(), +}); +export type SubagentModelPolicyWire = z.infer; + +export const subagentModelPolicyResponseSchema = z.object({ + policy: subagentModelPolicyWireSchema, + resource_version: z.string().min(1), + effective: z.object({ + configured_policy: subagentModelPolicyWireSchema, + effective_policy: subagentModelPolicyWireSchema, + policy_source: z.enum(['config', 'default']), + feature: z.object({ + enabled: z.boolean(), + source: z.enum(['master-env', 'env', 'config', 'default']), + }), + }), +}); +export type SubagentModelPolicyResponse = z.infer; + export const patchConfigRequestSchema = z.object({ providers: z.record(z.string(), z.unknown()).optional(), default_provider: z.string().optional(), diff --git a/packages/agent-gateway/src/routes/config.ts b/packages/agent-gateway/src/routes/config.ts index cb3dffe5e..73072b68f 100644 --- a/packages/agent-gateway/src/routes/config.ts +++ b/packages/agent-gateway/src/routes/config.ts @@ -3,6 +3,8 @@ import { IConfigRegistry, IConfigService, IEventService, + ISubagentModelPolicyService, + prospectiveModelView, type Scope, } from '@pymodel/agent-core-v2'; @@ -83,8 +85,12 @@ export function registerConfigRoutes(app: ConfigRouteHost, core: Scope): void { staged[domain] = registry.merge(domain, base, camelPatch[domain]); } if (secondaryModel !== undefined) { - staged[SECONDARY_MODEL_DOMAIN] = - secondaryModel === null ? null : toSecondaryModelReplacement(secondaryModel); + const preview = config.previewReplaceSections(staged); + const prepared = core.accessor.get(ISubagentModelPolicyService).prepareLegacyMutation( + secondaryModel === null ? null : toSecondaryModelReplacement(secondaryModel), + prospectiveModelView(preview['providers'], preview['models']), + ); + staged[SECONDARY_MODEL_DOMAIN] = prepared.section ?? null; } await config.replaceSections(staged); const response = toConfigResponse(config.getAll()); diff --git a/packages/agent-gateway/src/routes/registerApiV1Routes.ts b/packages/agent-gateway/src/routes/registerApiV1Routes.ts index fc56aa77c..761999e68 100644 --- a/packages/agent-gateway/src/routes/registerApiV1Routes.ts +++ b/packages/agent-gateway/src/routes/registerApiV1Routes.ts @@ -15,6 +15,7 @@ import { registerAuthRoute } from './auth'; import { registerCapabilitiesRoutes } from './capabilities'; import { registerCodexLoginRoutes } from './codex'; import { registerConfigRoutes } from './config'; +import { registerSubagentModelPolicyRoutes } from './subagentModelPolicy'; import { registerConnectionsRoutes } from './connections'; import { registerFilesRoutes } from './files'; import { registerFsRoutes } from './fs'; @@ -129,6 +130,10 @@ export async function registerApiV1Routes( core, ); registerConfigRoutes(apiV1 as unknown as Parameters[0], core); + registerSubagentModelPolicyRoutes( + apiV1 as unknown as Parameters[0], + core, + ); registerModelCatalogRoutes( apiV1 as unknown as Parameters[0], core, diff --git a/packages/agent-gateway/src/routes/subagentModelPolicy.ts b/packages/agent-gateway/src/routes/subagentModelPolicy.ts new file mode 100644 index 000000000..ab8614432 --- /dev/null +++ b/packages/agent-gateway/src/routes/subagentModelPolicy.ts @@ -0,0 +1,214 @@ +import { + ConfigChanged, + ErrorCodes, + IConfigService, + IEventService, + isError2, + ISubagentModelPolicyService, + type CanonicalSubagentModelPolicy, + type EffectiveSubagentModelPolicy, + type Scope, +} from '@pymodel/agent-core-v2'; +import { errEnvelope, okEnvelope } from '../envelope'; +import { requestLog } from '../lib/requestLog'; +import { defineRoute } from '../middleware/defineRoute'; +import { ErrorCode } from '../protocol/error-codes'; +import { + subagentModelPolicyRequestSchema, + subagentModelPolicyResponseSchema, + type SubagentModelPolicyRequest, + type SubagentModelPolicyResponse, + type SubagentModelPolicyWire, +} from '../protocol/rest-config'; + +interface PolicyRequest { + readonly id: string; + readonly body?: unknown; + readonly headers: Record; +} + +interface PolicyReply { + header(name: string, value: string): PolicyReply; + code(status: number): PolicyReply; + send(payload: unknown): unknown; +} + +interface PolicyRouteHost { + get(path: string, options: { schema?: Record }, handler: (req: PolicyRequest, reply: PolicyReply) => Promise | void): unknown; + put(path: string, options: { schema?: Record }, handler: (req: PolicyRequest, reply: PolicyReply) => Promise | void): unknown; + delete(path: string, options: { schema?: Record }, handler: (req: PolicyRequest, reply: PolicyReply) => Promise | void): unknown; +} + +export const SUBAGENT_MODEL_POLICY_PATH = '/config/subagent-model-policy'; + +export function toWirePolicy(policy: CanonicalSubagentModelPolicy): SubagentModelPolicyWire { + switch (policy.mode) { + case 'inherit': + return { mode: 'inherit' }; + case 'default': + return { mode: 'default', default_model: policy.defaultModel, default_effort: policy.defaultEffort }; + case 'pool': + return { + mode: 'pool', + default_model: policy.defaultModel, + models: policy.models, + default_effort: policy.defaultEffort, + }; + case 'force': + return { mode: 'force', default_model: policy.defaultModel, default_effort: policy.defaultEffort }; + } +} + +export function fromWirePolicy(wire: SubagentModelPolicyRequest): Record { + switch (wire.mode) { + case 'inherit': + return { mode: 'inherit' }; + case 'default': + return { mode: 'default', defaultModel: wire.default_model, defaultEffort: wire.default_effort }; + case 'pool': + return { + mode: 'pool', + defaultModel: wire.default_model, + models: wire.models, + defaultEffort: wire.default_effort, + }; + case 'force': + return { mode: 'force', defaultModel: wire.default_model, defaultEffort: wire.default_effort }; + } +} + +export function etagOf(resourceVersion: string): string { + return `"${resourceVersion}"`; +} + +export function parseIfMatch(header: unknown): string | undefined { + if (typeof header !== 'string') return undefined; + const trimmed = header.trim(); + if (trimmed.length === 0 || trimmed === '*') return undefined; + const first = trimmed.split(',')[0]?.trim() ?? ''; + const strong = first.startsWith('W/') ? first.slice(2) : first; + return strong.startsWith('"') && strong.endsWith('"') ? strong.slice(1, -1) : strong; +} + +function toResponse( + policy: CanonicalSubagentModelPolicy, + resourceVersion: string, + effective: EffectiveSubagentModelPolicy, +): SubagentModelPolicyResponse { + return { + policy: toWirePolicy(policy), + resource_version: resourceVersion, + effective: { + configured_policy: toWirePolicy(effective.configuredPolicy), + effective_policy: toWirePolicy(effective.effectivePolicy), + policy_source: effective.policySource, + feature: { enabled: effective.feature.enabled, source: effective.feature.source }, + }, + }; +} + +export function registerSubagentModelPolicyRoutes(app: PolicyRouteHost, core: Scope): void { + const policyService = (): ISubagentModelPolicyService => + core.accessor.get(ISubagentModelPolicyService); + + const respond = (reply: PolicyReply, requestId: string): void => { + const service = policyService(); + const snapshot = service.get(); + reply + .header('etag', etagOf(snapshot.resourceVersion)) + .send(okEnvelope(toResponse(snapshot.policy, snapshot.resourceVersion, service.getEffective()), requestId)); + }; + + const fail = (req: PolicyRequest, reply: PolicyReply, error: unknown): void => { + const message = error instanceof Error ? error.message : String(error); + if (isError2(error) && error.code === ErrorCodes.CONFIG_VERSION_CONFLICT) { + requestLog(req)?.info({ err: error }, 'subagent model policy version conflict'); + reply.code(412).send(errEnvelope(ErrorCode.CONFIG_VERSION_CONFLICT, message, req.id)); + return; + } + requestLog(req)?.error({ err: error }, 'subagent model policy update failed'); + reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, message, req.id)); + }; + + const publish = (changedFields: string[]): void => { + core.accessor.get(IEventService).publish( + new ConfigChanged({ + payload: { changedFields, config: core.accessor.get(IConfigService).getAll() }, + }), + ); + }; + + const getRoute = defineRoute( + { + method: 'GET', + path: SUBAGENT_MODEL_POLICY_PATH, + success: { data: subagentModelPolicyResponseSchema }, + description: 'Get the configured and effective subagent model routing policy', + tags: ['config'], + }, + async (req, reply) => { + await core.accessor.get(IConfigService).ready; + respond(reply as unknown as PolicyReply, req.id); + }, + ); + app.get(getRoute.path, getRoute.options, getRoute.handler as unknown as Parameters[2]); + + const putRoute = defineRoute( + { + method: 'PUT', + path: SUBAGENT_MODEL_POLICY_PATH, + body: subagentModelPolicyRequestSchema, + success: { data: subagentModelPolicyResponseSchema }, + errors: { + [ErrorCode.VALIDATION_FAILED]: {}, + [ErrorCode.CONFIG_VERSION_CONFLICT]: {}, + }, + description: 'Replace the subagent model routing policy (If-Match with the ETag from GET)', + tags: ['config'], + }, + async (req, reply) => { + const typedReply = reply as unknown as PolicyReply; + try { + await core.accessor.get(IConfigService).ready; + const expectedVersion = parseIfMatch(req.headers['if-match']); + await policyService().set(fromWirePolicy(req.body), expectedVersion); + } catch (error) { + fail(req as PolicyRequest, typedReply, error); + return; + } + publish(['secondary_model']); + requestLog(req)?.info({ changedFields: ['secondary_model'] }, 'subagent model policy updated'); + respond(typedReply, req.id); + }, + ); + app.put(putRoute.path, putRoute.options, putRoute.handler as unknown as Parameters[2]); + + const deleteRoute = defineRoute( + { + method: 'DELETE', + path: SUBAGENT_MODEL_POLICY_PATH, + success: { data: subagentModelPolicyResponseSchema }, + errors: { + [ErrorCode.VALIDATION_FAILED]: {}, + [ErrorCode.CONFIG_VERSION_CONFLICT]: {}, + }, + description: 'Remove the subagent model routing policy so subagents inherit the caller model', + tags: ['config'], + }, + async (req, reply) => { + const typedReply = reply as unknown as PolicyReply; + try { + await core.accessor.get(IConfigService).ready; + const expectedVersion = parseIfMatch(req.headers['if-match']); + await policyService().clear(expectedVersion); + } catch (error) { + fail(req as PolicyRequest, typedReply, error); + return; + } + publish(['secondary_model']); + requestLog(req)?.info({ changedFields: ['secondary_model'] }, 'subagent model policy cleared'); + respond(typedReply, req.id); + }, + ); + app.delete(deleteRoute.path, deleteRoute.options, deleteRoute.handler as unknown as Parameters[2]); +} diff --git a/packages/agent-gateway/test/__snapshots__/apiSurface.snapshot.test.ts.snap b/packages/agent-gateway/test/__snapshots__/apiSurface.snapshot.test.ts.snap index 4a2ee69a3..1ec4c1a38 100644 --- a/packages/agent-gateway/test/__snapshots__/apiSurface.snapshot.test.ts.snap +++ b/packages/agent-gateway/test/__snapshots__/apiSurface.snapshot.test.ts.snap @@ -20,6 +20,10 @@ exports[`API surface snapshot > matches the documented v2 route table and meta e ], ], "routes": [ + [ + "DELETE", + "/api/v1/config/subagent-model-policy", + ], [ "DELETE", "/api/v1/files/{file_id}", @@ -64,6 +68,10 @@ exports[`API surface snapshot > matches the documented v2 route table and meta e "GET", "/api/v1/config", ], + [ + "GET", + "/api/v1/config/subagent-model-policy", + ], [ "GET", "/api/v1/connections", @@ -516,6 +524,10 @@ exports[`API surface snapshot > matches the documented v2 route table and meta e "POST", "/api/v2/sessions:restore", ], + [ + "PUT", + "/api/v1/config/subagent-model-policy", + ], [ "PUT", "/api/v1/providers/{provider_id}", diff --git a/packages/agent-gateway/test/config.test.ts b/packages/agent-gateway/test/config.test.ts index 43263b098..61aef861f 100644 --- a/packages/agent-gateway/test/config.test.ts +++ b/packages/agent-gateway/test/config.test.ts @@ -100,8 +100,31 @@ describe('server-v2 /api/v1/config', () => { expect(after.yolo).toBe(false); }); + const FAST_MODELS_TOML = [ + '[providers.provider]', + 'type = "openai"', + 'api_key = "sk-test"', + 'base_url = "https://provider.example.test"', + '', + '[models."provider/fast"]', + 'provider = "provider"', + 'model = "fast"', + 'max_context_size = 1000', + '', + '[models."provider/fast_model"]', + 'provider = "provider"', + 'model = "fast-model"', + 'max_context_size = 1000', + '', + '[models."provider/slow"]', + 'provider = "provider"', + 'model = "slow"', + 'max_context_size = 1000', + '', + ].join('\n'); + it('POST { secondary_model } persists the subagent model pool and GET echoes it', async () => { - await boot(); + await boot(FAST_MODELS_TOML); const cfg = await patchConfig({ secondary_model: { default_model: 'provider/fast', @@ -118,7 +141,7 @@ describe('server-v2 /api/v1/config', () => { }); it('POST { secondary_model } preserves pool alias keys containing underscores', async () => { - await boot(); + await boot(FAST_MODELS_TOML); await patchConfig({ secondary_model: { default_model: 'provider/fast_model', models: { 'provider/fast_model': '' } }, }); @@ -134,7 +157,7 @@ describe('server-v2 /api/v1/config', () => { }); it('POST { secondary_model: null } removes the subagent model override', async () => { - await boot('[secondary_model]\nmodel = "provider/fast"\ndefault_effort = "low"\n'); + await boot(`${FAST_MODELS_TOML}[secondary_model]\nmodel = "provider/fast"\ndefault_effort = "low"\n`); const cfg = await patchConfig({ secondary_model: null }); expect(cfg.secondary_model).toBeUndefined(); @@ -238,8 +261,26 @@ describe('server-v2 /api/v1/config secondary_model replacement and request atomi return readFile(join(home as string, 'config.toml'), 'utf-8'); } + const MODELS_TOML = [ + '[providers.provider]', + 'type = "openai"', + 'api_key = "sk-test"', + 'base_url = "https://provider.example.test"', + '', + '[models."provider/fast"]', + 'provider = "provider"', + 'model = "fast"', + 'max_context_size = 1000', + '', + '[models."provider/slow"]', + 'provider = "provider"', + 'model = "slow"', + 'max_context_size = 1000', + '', + ].join('\n'); + it('force true -> false drops the force field instead of keeping the stale value', async () => { - await boot(); + await boot(MODELS_TOML); await post({ secondary_model: { default_model: 'provider/fast', force: true } }); expect((await getConfig()).secondary_model).toMatchObject({ force: true }); @@ -250,8 +291,23 @@ describe('server-v2 /api/v1/config secondary_model replacement and request atomi expect(await diskToml()).not.toContain('force'); }); + it('accepts legacy secondary_model metadata echoed by GET and drops it on write', async () => { + await boot( + `${MODELS_TOML}[secondary_model]\ndefault_model = "provider/fast"\nmax_context_size = 1000\ncapabilities = ["thinking"]\n`, + ); + const echoed = (await getConfig()).secondary_model as Record; + expect(echoed).toMatchObject({ defaultModel: 'provider/fast', maxContextSize: 1000 }); + + const res = await post({ secondary_model: { ...echoed, default_effort: 'low' } }); + expect(res.body.code).toBe(0); + expect((await getConfig()).secondary_model).toEqual({ + defaultModel: 'provider/fast', + defaultEffort: 'low', + }); + }); + it('pool -> default drops the models table', async () => { - await boot(); + await boot(MODELS_TOML); await post({ secondary_model: { default_model: 'provider/fast', @@ -264,7 +320,7 @@ describe('server-v2 /api/v1/config secondary_model replacement and request atomi }); it('pool -> force drops the models table and keeps force', async () => { - await boot(); + await boot(MODELS_TOML); await post({ secondary_model: { default_model: 'provider/fast', models: { 'provider/fast': '' } }, }); @@ -276,7 +332,7 @@ describe('server-v2 /api/v1/config secondary_model replacement and request atomi }); it('default -> inherit removes the section from disk', async () => { - await boot(); + await boot(MODELS_TOML); await post({ secondary_model: { default_model: 'provider/fast' } }); await post({ secondary_model: null }); expect((await getConfig()).secondary_model).toBeUndefined(); @@ -299,7 +355,7 @@ describe('server-v2 /api/v1/config secondary_model replacement and request atomi }); it('accepts the web client camelCase secondary_model shape and drops force: false', async () => { - await boot(); + await boot(MODELS_TOML); const res = await post({ secondary_model: { defaultModel: 'provider/fast', defaultEffort: 'low', force: false }, }); @@ -333,6 +389,64 @@ describe('server-v2 /api/v1/config secondary_model replacement and request atomi expect(await diskToml()).not.toContain('timeout_ms'); }); + it('validates secondary_model against the prospective post-request model configuration', async () => { + await boot(MODELS_TOML); + const luna = { provider: 'provider', model: 'luna', max_context_size: 1000 }; + + const unknown = await post({ secondary_model: { default_model: 'provider/luna' } }); + expect(unknown.body.code).toBe(ErrorCode.VALIDATION_FAILED); + expect(await diskToml()).not.toContain('secondary_model'); + + const together = await post({ + models: { 'provider/luna': luna }, + secondary_model: { default_model: 'provider/luna', models: { 'provider/luna': 'new' } }, + }); + expect(together.body.code).toBe(0); + expect((await getConfig()).secondary_model).toMatchObject({ defaultModel: 'provider/luna' }); + expect(await diskToml()).toContain('[models."provider/luna"]'); + + const mixedInvalid = await post({ + models: { 'provider/sol': { provider: 'provider', model: 'sol', max_context_size: 1000 } }, + secondary_model: { default_model: 'provider/sol', models: { 'provider/sol': '', 'provider/nope': '' } }, + }); + expect(mixedInvalid.body.code).toBe(ErrorCode.VALIDATION_FAILED); + expect(await diskToml()).not.toContain('[models."provider/sol"]'); + expect((await getConfig()).secondary_model).toMatchObject({ defaultModel: 'provider/luna' }); + + const swap = await post({ + models: { 'provider/sol': { provider: 'provider', model: 'sol', max_context_size: 1000 } }, + secondary_model: { default_model: 'provider/sol' }, + }); + expect(swap.body.code).toBe(0); + expect(await diskToml()).toContain('[models."provider/sol"]'); + expect((await getConfig()).secondary_model).toEqual({ defaultModel: 'provider/sol' }); + }); + + it('validates secondary_model against the effective configuration including the env model overlay', async () => { + await boot(MODELS_TOML, { + ...process.env, + PYTHINKER_MODEL_NAME: 'env-model', + PYTHINKER_API_KEY: 'sk-env', + PYTHINKER_BASE_URL: 'https://env.example.test', + }); + const cfg = await getConfig(); + expect(Object.keys(cfg.models ?? {})).toContain('__pythinker_env_model__'); + + const envAlias = await post({ secondary_model: { default_model: '__pythinker_env_model__' } }); + expect(envAlias.body.code).toBe(0); + expect((await getConfig()).secondary_model).toEqual({ defaultModel: '__pythinker_env_model__' }); + + const both = await post({ + models: { 'provider/extra': { provider: 'provider', model: 'extra', max_context_size: 1000 } }, + secondary_model: { + default_model: '__pythinker_env_model__', + models: { '__pythinker_env_model__': '', 'provider/fast': '', 'provider/extra': '' }, + }, + }); + expect(both.body.code).toBe(0); + expect((await getConfig()).secondary_model).toMatchObject({ defaultModel: '__pythinker_env_model__' }); + }); + it('rejects a malformed secondary_model body with VALIDATION_FAILED and writes nothing', async () => { await boot(); const res = await post({ secondary_model: { default_model: 42, force: 'yes' } }); diff --git a/packages/agent-gateway/test/subagentModelPolicy.test.ts b/packages/agent-gateway/test/subagentModelPolicy.test.ts new file mode 100644 index 000000000..2c0cded02 --- /dev/null +++ b/packages/agent-gateway/test/subagentModelPolicy.test.ts @@ -0,0 +1,254 @@ +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { IConfigService } from '@pymodel/agent-core-v2'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { ErrorCode } from '../src/protocol/error-codes'; +import { + subagentModelPolicyResponseSchema, + type SubagentModelPolicyResponse, +} from '../src/protocol/rest-config'; +import { type RunningServer, startServer } from '../src/start'; +import { TEST_HOST_IDENTITY } from './helpers/hostIdentity'; +import { authedFetch } from './helpers/auth'; + +interface Envelope { + code: number; + msg: string; + data: T; +} + +const PATH = '/api/v1/config/subagent-model-policy'; + +const MODELS_TOML = [ + '[providers.acme]', + 'type = "openai"', + 'api_key = "sk-test"', + 'base_url = "https://acme.example.test"', + '', + '[models."acme/sol"]', + 'provider = "acme"', + 'model = "sol"', + 'max_context_size = 100000', + '', + '[models."acme/luna"]', + 'provider = "acme"', + 'model = "luna"', + 'max_context_size = 100000', + '', +].join('\n'); + +describe('server-v2 /api/v1/config/subagent-model-policy', () => { + let server: RunningServer | undefined; + let home: string | undefined; + let base: string; + + beforeEach(async () => { + vi.stubEnv('PYTHINKER_CODE_EXPERIMENTAL_FLAG', '0'); + vi.stubEnv('PYTHINKER_CODE_EXPERIMENTAL_SECONDARY_MODEL', undefined); + home = await mkdtemp(join(tmpdir(), 'pythinker-server-v2-policy-')); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + if (server !== undefined) { + await server.close(); + server = undefined; + } + if (home !== undefined) { + await rm(home, { recursive: true, force: true }); + home = undefined; + } + }); + + async function boot(toml: string = MODELS_TOML): Promise { + await writeFile(join(home as string, 'config.toml'), toml, 'utf-8'); + server = await startServer({ + hostIdentity: TEST_HOST_IDENTITY, + host: '127.0.0.1', + port: 0, + homeDir: home, + logLevel: 'silent', + }); + base = `http://127.0.0.1:${server.port}`; + } + + async function call( + method: 'GET' | 'PUT' | 'DELETE', + body?: unknown, + headers: Record = {}, + ): Promise<{ status: number; etag: string | null; body: Envelope }> { + const res = await authedFetch(server as RunningServer, base, PATH, { + method, + headers: body === undefined ? headers : { 'content-type': 'application/json', ...headers }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + return { + status: res.status, + etag: res.headers.get('etag'), + body: (await res.json()) as Envelope, + }; + } + + async function disk(): Promise { + return readFile(join(home as string, 'config.toml'), 'utf-8'); + } + + it('GET on the absent section returns inherit with a strong ETag', async () => { + await boot(); + const res = await call('GET'); + expect(res.status).toBe(200); + expect(res.body.code).toBe(0); + const data = subagentModelPolicyResponseSchema.parse(res.body.data); + expect(data.policy).toEqual({ mode: 'inherit' }); + expect(data.effective.effective_policy).toEqual({ mode: 'inherit' }); + expect(data.effective.feature).toEqual({ enabled: false, source: 'default' }); + expect(res.etag).toMatch(/^"subagent-policy-v1:[0-9a-f]+"$/); + expect(res.etag?.startsWith('W/')).toBe(false); + expect(res.etag).toBe(`"${data.resource_version}"`); + }); + + it('PUT persists a canonical policy, echoes a new ETag, and rejects unknown models before writing', async () => { + await boot(); + const before = (await call('GET')).etag; + const bad = await call('PUT', { mode: 'default', default_model: 'acme/nope' }); + expect(bad.status).toBe(200); + expect(bad.body.code).toBe(ErrorCode.VALIDATION_FAILED); + expect(await disk()).not.toContain('secondary_model'); + + const ok = await call('PUT', { + mode: 'pool', + default_model: 'acme/sol', + models: { 'acme/sol': 'main', 'acme/luna': 'fast' }, + default_effort: 'low', + }); + expect(ok.body.code).toBe(0); + expect(ok.etag).not.toBe(before); + const data = subagentModelPolicyResponseSchema.parse(ok.body.data); + expect(data.policy).toEqual({ + mode: 'pool', + default_model: 'acme/sol', + models: { 'acme/sol': 'main', 'acme/luna': 'fast' }, + default_effort: 'low', + }); + expect(await disk()).toContain('[secondary_model.models]'); + + const poolDefaultOutside = await call('PUT', { + mode: 'pool', + default_model: 'acme/luna', + models: { 'acme/sol': '' }, + }); + expect(poolDefaultOutside.body.code).toBe(ErrorCode.VALIDATION_FAILED); + expect((await call('GET')).etag).toBe(ok.etag); + }); + + it('If-Match with a stale ETag is rejected with 412 and leaves disk unchanged', async () => { + await boot(); + const first = await call('PUT', { mode: 'default', default_model: 'acme/sol' }); + const stale = first.etag as string; + const second = await call('PUT', { mode: 'default', default_model: 'acme/luna' }, { 'if-match': stale }); + expect(second.body.code).toBe(0); + const conflict = await call('PUT', { mode: 'force', default_model: 'acme/sol' }, { 'if-match': stale }); + expect(conflict.status).toBe(412); + expect(conflict.body.code).toBe(ErrorCode.CONFIG_VERSION_CONFLICT); + expect(await disk()).toContain('default_model = "acme/luna"'); + expect(await disk()).not.toContain('force'); + + const staleDelete = await call('DELETE', undefined, { 'if-match': stale }); + expect(staleDelete.status).toBe(412); + expect(await disk()).toContain('default_model = "acme/luna"'); + }); + + it('DELETE removes the section (no mode on disk) and a stale inherit ETag is rejected after another write', async () => { + await boot(); + const inheritEtag = (await call('GET')).etag as string; + await call('PUT', { mode: 'default', default_model: 'acme/sol' }); + const conflict = await call('PUT', { mode: 'inherit' }, { 'if-match': inheritEtag }); + expect(conflict.status).toBe(412); + + const current = (await call('GET')).etag as string; + const removed = await call('DELETE', undefined, { 'if-match': current }); + expect(removed.body.code).toBe(0); + expect(removed.body.data?.policy).toEqual({ mode: 'inherit' }); + expect(await disk()).not.toContain('secondary_model'); + expect(await disk()).not.toContain('mode ='); + expect((await call('GET')).etag).toBe(inheritEtag); + }); + + it('ETag is stable across key order and no-op writes, and changes on an external file edit', async () => { + await boot(); + const a = await call('PUT', { + mode: 'pool', + default_model: 'acme/sol', + models: { 'acme/sol': '', 'acme/luna': '' }, + }); + const b = await call('PUT', { + models: { 'acme/luna': '', 'acme/sol': '' }, + default_model: 'acme/sol', + mode: 'pool', + }); + expect(b.etag).toBe(a.etag); + const noop = await call('PUT', { + mode: 'pool', + default_model: 'acme/sol', + models: { 'acme/sol': '', 'acme/luna': '' }, + }, { 'if-match': a.etag as string }); + expect(noop.etag).toBe(a.etag); + + const edited = (await disk()).replace('[secondary_model.models]', '[secondary_model.models]\n"acme/extra" = ""'); + await writeFile(join(home as string, 'config.toml'), edited, 'utf-8'); + await (server as RunningServer).core.accessor.get(IConfigService).reload(); + expect((await call('GET')).etag).not.toBe(a.etag); + }); + + it('legacy POST /config and canonical PUT produce identical persisted state', async () => { + await boot(); + await call('PUT', { mode: 'force', default_model: 'acme/sol', default_effort: 'low' }); + const viaPut = await disk(); + expect(viaPut).toContain('[secondary_model]'); + const removed = await call('DELETE'); + expect(removed.body.code).toBe(0); + expect(removed.body.data?.policy).toEqual({ mode: 'inherit' }); + expect(await disk()).not.toContain('secondary_model'); + const legacy = await authedFetch(server as RunningServer, base, '/api/v1/config', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ secondary_model: { default_model: 'acme/sol', force: true, default_effort: 'low' } }), + }); + expect(((await legacy.json()) as Envelope).code).toBe(0); + expect(await disk()).toBe(viaPut); + expect((await call('GET')).body.data?.policy).toEqual({ + mode: 'force', + default_model: 'acme/sol', + default_effort: 'low', + }); + }); + + it('reports the effective policy as inherit while the feature is disabled and the configured one otherwise', async () => { + await boot(); + await call('PUT', { mode: 'force', default_model: 'acme/sol' }); + const disabled = subagentModelPolicyResponseSchema.parse((await call('GET')).body.data); + expect(disabled.effective.configured_policy.mode).toBe('force'); + expect(disabled.effective.effective_policy).toEqual({ mode: 'inherit' }); + expect(disabled.effective.policy_source).toBe('default'); + + await authedFetch(server as RunningServer, base, '/api/v1/config', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ experimental: { 'secondary-model': true } }), + }); + const enabled = subagentModelPolicyResponseSchema.parse((await call('GET')).body.data); + expect(enabled.effective.effective_policy.mode).toBe('force'); + expect(enabled.effective.feature).toEqual({ enabled: true, source: 'config' }); + }); + + it('rejects a malformed body with VALIDATION_FAILED', async () => { + await boot(); + const res = await call('PUT', { mode: 'pool', default_model: 'acme/sol' }); + expect(res.body.code).toBe(ErrorCode.VALIDATION_FAILED); + const unknownMode = await call('PUT', { mode: 'sometimes' }); + expect(unknownMode.body.code).toBe(ErrorCode.VALIDATION_FAILED); + }); +}); diff --git a/packages/oauth/src/refreshProviderModels.ts b/packages/oauth/src/refreshProviderModels.ts index b6863541e..1955f7004 100644 --- a/packages/oauth/src/refreshProviderModels.ts +++ b/packages/oauth/src/refreshProviderModels.ts @@ -319,6 +319,29 @@ function clampDanglingDefault(config: PythinkerConfigShape): void { } } +// The same refresh can drop a model that `[secondary_model]` binds. A dangling +// default binding clears the section (subagents inherit the caller model again); +// a dangling pool entry is pruned so the rest of the pool keeps working. The +// discovery service validates the section against the refreshed catalog and +// would otherwise reject the whole provider patch. +function clampDanglingSecondaryModel(config: PythinkerConfigShape): void { + const section = config.secondaryModel; + if (section === undefined) return; + for (const bound of [section.defaultModel, section.model]) { + if (bound !== undefined && readModel(config, bound) === undefined) { + config.secondaryModel = undefined; + return; + } + } + if (section.models === undefined) return; + const models = Object.fromEntries( + Object.entries(section.models).filter(([alias]) => readModel(config, alias) !== undefined), + ); + if (Object.keys(models).length !== Object.keys(section.models).length) { + config.secondaryModel = { ...section, models }; + } +} + function clearDefaultThinkingWhenDefaultRemoved( config: PythinkerConfigShape, previousDefaultModel: string | undefined, @@ -433,6 +456,7 @@ export async function refreshProviderModels( preserveSecondaryModelAliases(config, next); restoreDefaultSelection(next, config.defaultModel, config.thinking?.enabled); clampDanglingDefault(next); + clampDanglingSecondaryModel(next); clearDefaultThinkingWhenDefaultRemoved(next, config.defaultModel); if (providerModelsEqual(config, next, providerId, refreshedAliasKeys)) { @@ -584,6 +608,7 @@ export async function refreshProviderModels( if (changedProviders.length > 0 || hasUnreportedConfigChange) { restoreDefaultSelection(next, config.defaultModel, config.thinking?.enabled); clampDanglingDefault(next); + clampDanglingSecondaryModel(next); clearDefaultThinkingWhenDefaultRemoved(next, config.defaultModel); for (const providerId of providersToRemoveBeforeSet) { await host.removeProvider(providerId); @@ -695,6 +720,7 @@ export async function refreshProviderModels( if (changedProviders.length > 0) { restoreDefaultSelection(next, config.defaultModel, config.thinking?.enabled); clampDanglingDefault(next); + clampDanglingSecondaryModel(next); clearDefaultThinkingWhenDefaultRemoved(next, config.defaultModel); for (const providerId of providersToRemoveBeforeSet) { await host.removeProvider(providerId); diff --git a/packages/oauth/test/models-dev-refresh.test.ts b/packages/oauth/test/models-dev-refresh.test.ts index d522b94c7..6e2ef0491 100644 --- a/packages/oauth/test/models-dev-refresh.test.ts +++ b/packages/oauth/test/models-dev-refresh.test.ts @@ -238,7 +238,46 @@ describe('refreshProviderModels modelsDev directory providers', () => { expect(patch.providers?.[PROVIDER_ID]).toBeUndefined(); expect(patch.defaultModel).toBeUndefined(); expect(patch.thinking).toBeUndefined(); - expect(patch.secondaryModel).toEqual(base.secondaryModel); + expect(patch.secondaryModel).toBeUndefined(); + }); + + it('clears secondary_model when only the legacy model key dangles beside a valid default_model', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => jsonResponse({ 'brand-new-guy': makeDocument()['brand-new-guy'] })), + ); + const base = makeBaseConfig(); + base.models = { ...base.models, 'other/kept': { provider: 'other', model: 'kept' } }; + base.secondaryModel = { defaultModel: 'other/kept', model: `${PROVIDER_ID}/deepseek-v4-flash` }; + + const { host, calls } = makeHost(base); + await refreshProviderModels(host); + + expect(lastPatch(calls).secondaryModel).toBeUndefined(); + }); + + it('prunes vanished pool entries from secondary_model but keeps the rest of the pool', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => jsonResponse({ 'brand-new-guy': makeDocument()['brand-new-guy'] })), + ); + const base = makeBaseConfig(); + base.models = { + ...base.models, + 'other/kept': { provider: 'other', model: 'kept' }, + }; + base.secondaryModel = { + defaultModel: 'other/kept', + models: { 'other/kept': '', [`${PROVIDER_ID}/deepseek-v4-flash`]: 'fast' }, + }; + + const { host, calls } = makeHost(base); + await refreshProviderModels(host); + + expect(lastPatch(calls).secondaryModel).toEqual({ + defaultModel: 'other/kept', + models: { 'other/kept': '' }, + }); }); it('reports a failure without writing when an entry lists no usable models', async () => {