mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-08-22 07:05:41 +00:00
fix(agent-core): strip the no-op subagent model parameter while the secondary-model experiment is off (#2449)
The Agent/AgentSwarm tool schemas always advertised a \`model\` choice parameter, so the secondary-model concept entered the prompt even with the experiment disabled. Gate the advertised JSON schema on the flag in both engines: off (the default) drops the parameter, on keeps it, and spawn-time resolution already falls back to the caller's model either way. Also scrub ambient KIMI_CODE_EXPERIMENTAL_* env vars in both packages' vitest setup so flag-dependent tool schemas in llm.tools_snapshot stay deterministic regardless of the developer shell.
This commit is contained in:
parent
ed7a4cc095
commit
95a656ca61
15 changed files with 241 additions and 14 deletions
|
|
@ -45,7 +45,9 @@ import {
|
|||
buildSubagentModelDescriptions,
|
||||
resolveSubagentBinding,
|
||||
resolveSubagentTimeoutMs,
|
||||
stripSubagentModelParameter,
|
||||
} from '#/session/subagent/configSection';
|
||||
import { SECONDARY_MODEL_FLAG_ID } from '#/session/subagent/flag';
|
||||
import {
|
||||
AgentSwarmToolInputSchema,
|
||||
IAgentSwarmTool,
|
||||
|
|
@ -57,6 +59,9 @@ import AGENT_SWARM_DESCRIPTION from './agent-swarm.md?raw';
|
|||
|
||||
const DEFAULT_SUBAGENT_TYPE = 'coder';
|
||||
|
||||
const AGENT_SWARM_PARAMETERS = toInputJsonSchema(AgentSwarmToolInputSchema);
|
||||
const AGENT_SWARM_PARAMETERS_NO_MODEL = stripSubagentModelParameter(AGENT_SWARM_PARAMETERS);
|
||||
|
||||
interface AgentSwarmSpawnSpec {
|
||||
readonly kind: 'spawn';
|
||||
readonly index: number;
|
||||
|
|
@ -86,7 +91,17 @@ interface SwarmRunResult {
|
|||
export class AgentSwarmTool implements IAgentSwarmTool {
|
||||
declare readonly _serviceBrand: undefined;
|
||||
readonly name = 'AgentSwarm' as const;
|
||||
readonly parameters: Record<string, unknown> = toInputJsonSchema(AgentSwarmToolInputSchema);
|
||||
|
||||
/**
|
||||
* The `model` choice only exists while the `secondary-model` experiment is
|
||||
* on; off, the advertised schema drops it so the concept never enters the
|
||||
* prompt. Read live per request (same as `description`).
|
||||
*/
|
||||
get parameters(): Record<string, unknown> {
|
||||
return this.flags.enabled(SECONDARY_MODEL_FLAG_ID)
|
||||
? AGENT_SWARM_PARAMETERS
|
||||
: AGENT_SWARM_PARAMETERS_NO_MODEL;
|
||||
}
|
||||
|
||||
private readonly callerAgentId: string;
|
||||
|
||||
|
|
|
|||
|
|
@ -84,6 +84,7 @@ import {
|
|||
formatSubagentTimeoutDescription,
|
||||
resolveSubagentBinding,
|
||||
resolveSubagentTimeoutMs,
|
||||
stripSubagentModelParameter,
|
||||
wrapSubagentModelError,
|
||||
} from '#/session/subagent/configSection';
|
||||
import { SECONDARY_MODEL_FLAG_ID } from '#/session/subagent/flag';
|
||||
|
|
@ -104,10 +105,23 @@ import AGENT_BACKGROUND_DISABLED_DESCRIPTION from './agent-background-disabled.m
|
|||
import AGENT_BACKGROUND_DESCRIPTION from './agent-background-enabled.md?raw';
|
||||
import AGENT_DESCRIPTION_BASE from './agent.md?raw';
|
||||
|
||||
const SUBAGENT_TOOL_PARAMETERS = toInputJsonSchema(SubagentToolInputSchema);
|
||||
const SUBAGENT_TOOL_PARAMETERS_NO_MODEL = stripSubagentModelParameter(SUBAGENT_TOOL_PARAMETERS);
|
||||
|
||||
export class SubagentTool implements ISubagentTool {
|
||||
declare readonly _serviceBrand: undefined;
|
||||
readonly name: string = 'Agent';
|
||||
readonly parameters: Record<string, unknown> = toInputJsonSchema(SubagentToolInputSchema);
|
||||
|
||||
/**
|
||||
* The `model` choice only exists while the `secondary-model` experiment is
|
||||
* on; off, the advertised schema drops it so the concept never enters the
|
||||
* prompt. Read live per request (same as `description`).
|
||||
*/
|
||||
get parameters(): Record<string, unknown> {
|
||||
return this.flags.enabled(SECONDARY_MODEL_FLAG_ID)
|
||||
? SUBAGENT_TOOL_PARAMETERS
|
||||
: SUBAGENT_TOOL_PARAMETERS_NO_MODEL;
|
||||
}
|
||||
|
||||
private readonly callerAgentId: string;
|
||||
private readonly canRunInBackground: () => boolean;
|
||||
|
|
|
|||
|
|
@ -25,7 +25,9 @@
|
|||
* rather than inheriting the caller's level. Both tools resolve spawn
|
||||
* bindings through `resolveSubagentBinding`, advertise the pair via
|
||||
* `buildSubagentModelDescriptions`, and wrap spawn failures with
|
||||
* `wrapSubagentModelError`. Self-registered at module load via
|
||||
* `wrapSubagentModelError`; while the experiment is off they also strip the
|
||||
* no-op `model` parameter from their advertised schemas via
|
||||
* `stripSubagentModelParameter`. Self-registered at module load via
|
||||
* `registerConfigSection`, so the `config` domain never imports this
|
||||
* domain's types.
|
||||
*/
|
||||
|
|
@ -34,6 +36,7 @@ import { z } from 'zod';
|
|||
|
||||
import { Error2, ErrorCodes, isError2 } from '#/errors';
|
||||
import type { AgentModelPreference } from '#/app/agentProfileCatalog/agentProfileCatalog';
|
||||
import { isPlainObject } from '#/app/config/toml';
|
||||
import type { IFlagService } from '#/app/flag/flag';
|
||||
import {
|
||||
SECONDARY_MODEL_ENV,
|
||||
|
|
@ -143,6 +146,32 @@ export function buildSubagentModelDescriptions(
|
|||
].join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip the `model` property from a subagent collaboration tool's advertised
|
||||
* JSON schema. While the `secondary-model` experiment is off the parameter is
|
||||
* a silent no-op, so the schema the model sees (and the args validator
|
||||
* compiled from the same advertised schema) drops it entirely — the
|
||||
* secondary-model concept never enters the prompt, and a stray `model`
|
||||
* argument is rejected instead of silently inheriting the caller's model.
|
||||
* Returns the input unchanged when there is no `model` property; otherwise a
|
||||
* shallow copy — the input is never mutated, so callers can keep both
|
||||
* variants as shared constants.
|
||||
*/
|
||||
export function stripSubagentModelParameter(
|
||||
parameters: Record<string, unknown>,
|
||||
): Record<string, unknown> {
|
||||
const properties = parameters['properties'];
|
||||
if (!isPlainObject(properties) || !('model' in properties)) return parameters;
|
||||
const nextProperties = { ...properties };
|
||||
delete nextProperties['model'];
|
||||
const next: Record<string, unknown> = { ...parameters, properties: nextProperties };
|
||||
const required = parameters['required'];
|
||||
if (Array.isArray(required) && required.includes('model')) {
|
||||
next['required'] = required.filter((entry) => entry !== 'model');
|
||||
}
|
||||
return next;
|
||||
}
|
||||
|
||||
export function wrapSubagentModelError(
|
||||
error: unknown,
|
||||
boundModel: string,
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
13
packages/agent-core-v2/test/setup.ts
Normal file
13
packages/agent-core-v2/test/setup.ts
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
/**
|
||||
* Hermetic experimental-flag state for tests: scrub ambient
|
||||
* `KIMI_CODE_EXPERIMENTAL_*` env vars inherited from the developer shell
|
||||
* (e.g. a globally exported `KIMI_CODE_EXPERIMENTAL_FLAG=1`) so flag-driven
|
||||
* behavior — including tool schemas embedded in `llm.tools_snapshot`
|
||||
* snapshots — stays deterministic and matches CI. Tests opt into flags
|
||||
* explicitly via service overrides or `vi.stubEnv`.
|
||||
*/
|
||||
for (const key of Object.keys(process.env)) {
|
||||
if (key.startsWith('KIMI_CODE_EXPERIMENTAL_')) {
|
||||
delete process.env[key];
|
||||
}
|
||||
}
|
||||
File diff suppressed because one or more lines are too long
|
|
@ -4,5 +4,6 @@ export default defineConfig({
|
|||
test: {
|
||||
name: 'agent-core-v2',
|
||||
include: ['test/**/*.{test,e2e,integration}.ts'],
|
||||
setupFiles: ['test/setup.ts'],
|
||||
},
|
||||
});
|
||||
|
|
|
|||
|
|
@ -848,6 +848,7 @@ export class ToolManager {
|
|||
log: this.agent.log,
|
||||
subagentTimeoutMs: resolveSubagentTimeoutMs(this.agent.kimiConfig?.subagent?.timeoutMs),
|
||||
showModelPreferences: this.agent.experimentalFlags.enabled('secondary-model'),
|
||||
modelChoiceEnabled: this.agent.experimentalFlags.enabled('secondary-model'),
|
||||
subagentModelDescription: buildSubagentModelDescriptions(
|
||||
this.agent.kimiConfig,
|
||||
this.agent.experimentalFlags,
|
||||
|
|
@ -865,6 +866,7 @@ export class ToolManager {
|
|||
this.agent.experimentalFlags,
|
||||
this.agent.config.modelAlias,
|
||||
),
|
||||
this.agent.experimentalFlags.enabled('secondary-model'),
|
||||
),
|
||||
toolServices?.webSearcher && new b.WebSearchTool(toolServices.webSearcher),
|
||||
toolServices?.urlFetcher && new b.FetchURLTool(toolServices.urlFetcher),
|
||||
|
|
|
|||
|
|
@ -84,6 +84,34 @@ export function buildSubagentModelDescriptions(
|
|||
].join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip the `model` property from a subagent collaboration tool's advertised
|
||||
* JSON schema. While the `secondary-model` experiment is off the parameter is
|
||||
* a silent no-op, so the schema the model sees (and the args validator
|
||||
* compiled from the same advertised schema) drops it entirely — the
|
||||
* secondary-model concept never enters the prompt, and a stray `model`
|
||||
* argument is rejected instead of silently inheriting the caller's model.
|
||||
* Returns the input unchanged when there is no `model` property; otherwise a
|
||||
* shallow copy — the input is never mutated, so callers can keep both
|
||||
* variants as shared constants.
|
||||
*/
|
||||
export function stripSubagentModelParameter(
|
||||
parameters: Record<string, unknown>,
|
||||
): Record<string, unknown> {
|
||||
const properties = parameters['properties'];
|
||||
if (typeof properties !== 'object' || properties === null || !('model' in properties)) {
|
||||
return parameters;
|
||||
}
|
||||
const nextProperties = { ...(properties as Record<string, unknown>) };
|
||||
delete nextProperties['model'];
|
||||
const next: Record<string, unknown> = { ...parameters, properties: nextProperties };
|
||||
const required = parameters['required'];
|
||||
if (Array.isArray(required) && required.includes('model')) {
|
||||
next['required'] = required.filter((entry) => entry !== 'model');
|
||||
}
|
||||
return next;
|
||||
}
|
||||
|
||||
/**
|
||||
* Point a spawn-time model resolution failure at the secondary-model
|
||||
* configuration when the bound model is not the caller's own — otherwise the
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import {
|
|||
type QueuedSubagentTask,
|
||||
type SessionSubagentHost,
|
||||
} from '../../../session/subagent-host';
|
||||
import { stripSubagentModelParameter } from '../../../session/subagent-binding';
|
||||
import { ToolAccesses } from '../../../loop/tool-access';
|
||||
import type { ExecutableToolContext, ExecutableToolResult, ToolExecution } from '../../../loop/types';
|
||||
import { toInputJsonSchema } from '../../support/input-schema';
|
||||
|
|
@ -89,10 +90,13 @@ interface SwarmRunResult {
|
|||
readonly error?: string;
|
||||
}
|
||||
|
||||
const AGENT_SWARM_PARAMETERS = toInputJsonSchema(AgentSwarmToolInputSchema);
|
||||
const AGENT_SWARM_PARAMETERS_NO_MODEL = stripSubagentModelParameter(AGENT_SWARM_PARAMETERS);
|
||||
|
||||
export class AgentSwarmTool implements BuiltinTool<AgentSwarmToolInput> {
|
||||
readonly name = 'AgentSwarm' as const;
|
||||
readonly description: string;
|
||||
readonly parameters: Record<string, unknown> = toInputJsonSchema(AgentSwarmToolInputSchema);
|
||||
readonly parameters: Record<string, unknown>;
|
||||
|
||||
constructor(
|
||||
private readonly subagentHost: SessionSubagentHost,
|
||||
|
|
@ -101,11 +105,18 @@ export class AgentSwarmTool implements BuiltinTool<AgentSwarmToolInput> {
|
|||
// SubagentBatch arms no timer for non-positive timeouts.
|
||||
private readonly subagentTimeoutMs?: number,
|
||||
subagentModelDescription?: string,
|
||||
// Mirrors the `secondary-model` experiment: off (the default), the no-op
|
||||
// `model` parameter is stripped from the advertised schema so the
|
||||
// secondary-model concept never enters the prompt.
|
||||
modelChoiceEnabled = false,
|
||||
) {
|
||||
this.description =
|
||||
subagentModelDescription === undefined
|
||||
? AGENT_SWARM_DESCRIPTION
|
||||
: `${AGENT_SWARM_DESCRIPTION}\n\n${subagentModelDescription}`;
|
||||
this.parameters = modelChoiceEnabled
|
||||
? AGENT_SWARM_PARAMETERS
|
||||
: AGENT_SWARM_PARAMETERS_NO_MODEL;
|
||||
}
|
||||
|
||||
resolveExecution(args: AgentSwarmToolInput): ToolExecution {
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ import {
|
|||
type SessionSubagentHost,
|
||||
type SubagentHandle,
|
||||
} from '../../../session/subagent-host';
|
||||
import { stripSubagentModelParameter } from '../../../session/subagent-binding';
|
||||
import { isUserCancellation } from '../../../utils/abort';
|
||||
import { AgentBackgroundTask, type BackgroundManager } from '../../../agent/background';
|
||||
import { toInputJsonSchema } from '../../support/input-schema';
|
||||
|
|
@ -110,10 +111,13 @@ const BACKGROUND_AGENT_UNAVAILABLE =
|
|||
|
||||
// ── AgentTool class ──────────────────────────────────────────────────
|
||||
|
||||
const AGENT_TOOL_PARAMETERS = toInputJsonSchema(AgentToolInputSchema);
|
||||
const AGENT_TOOL_PARAMETERS_NO_MODEL = stripSubagentModelParameter(AGENT_TOOL_PARAMETERS);
|
||||
|
||||
export class AgentTool implements BuiltinTool<AgentToolInput> {
|
||||
readonly name: string = 'Agent';
|
||||
readonly description: string;
|
||||
readonly parameters: Record<string, unknown> = toInputJsonSchema(AgentToolInputSchema);
|
||||
readonly parameters: Record<string, unknown>;
|
||||
constructor(
|
||||
private readonly subagentHost: SessionSubagentHost,
|
||||
private readonly backgroundManager: BackgroundManager,
|
||||
|
|
@ -124,6 +128,10 @@ export class AgentTool implements BuiltinTool<AgentToolInput> {
|
|||
subagentTimeoutMs?: number | undefined;
|
||||
subagentModelDescription?: string;
|
||||
showModelPreferences?: boolean;
|
||||
// Mirrors the `secondary-model` experiment: off (the default), the
|
||||
// no-op `model` parameter is stripped from the advertised schema so the
|
||||
// secondary-model concept never enters the prompt.
|
||||
modelChoiceEnabled?: boolean;
|
||||
},
|
||||
) {
|
||||
const log = options?.log;
|
||||
|
|
@ -131,6 +139,10 @@ export class AgentTool implements BuiltinTool<AgentToolInput> {
|
|||
// `0` is preserved (not normalized): `0 ?? DEFAULT_SUBAGENT_TIMEOUT_MS`
|
||||
// stays `0`, and the BackgroundManager arms no timer for it.
|
||||
this.subagentTimeoutMs = options?.subagentTimeoutMs;
|
||||
this.parameters =
|
||||
options?.modelChoiceEnabled === true
|
||||
? AGENT_TOOL_PARAMETERS
|
||||
: AGENT_TOOL_PARAMETERS_NO_MODEL;
|
||||
const typeLines = buildSubagentDescriptions(
|
||||
subagents,
|
||||
options?.showModelPreferences ?? false,
|
||||
|
|
|
|||
13
packages/agent-core/test/setup.ts
Normal file
13
packages/agent-core/test/setup.ts
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
/**
|
||||
* Hermetic experimental-flag state for tests: scrub ambient
|
||||
* `KIMI_CODE_EXPERIMENTAL_*` env vars inherited from the developer shell
|
||||
* (e.g. a globally exported `KIMI_CODE_EXPERIMENTAL_FLAG=1`) so flag-driven
|
||||
* behavior — including tool schemas embedded in `llm.tools_snapshot`
|
||||
* snapshots — stays deterministic and matches CI. Tests opt into flags
|
||||
* explicitly via `vi.stubEnv` or harness flag options.
|
||||
*/
|
||||
for (const key of Object.keys(process.env)) {
|
||||
if (key.startsWith('KIMI_CODE_EXPERIMENTAL_')) {
|
||||
delete process.env[key];
|
||||
}
|
||||
}
|
||||
|
|
@ -136,9 +136,11 @@ describe('AgentTool', () => {
|
|||
expect(tool.description).toContain('Default to a foreground subagent');
|
||||
});
|
||||
|
||||
it('exposes a primary/secondary model parameter in the JSON schema', () => {
|
||||
it('exposes a primary/secondary model parameter in the JSON schema when the experiment is enabled', () => {
|
||||
const host = mockSubagentHost({ spawn: vi.fn() });
|
||||
const tool = agentTool(host);
|
||||
const tool = agentTool(host, createBackgroundManager().manager, undefined, {
|
||||
modelChoiceEnabled: true,
|
||||
});
|
||||
const properties = (
|
||||
tool.parameters as {
|
||||
properties: Record<string, { description?: string; enum?: string[] }>;
|
||||
|
|
@ -149,6 +151,15 @@ describe('AgentTool', () => {
|
|||
expect(properties['model']?.description).toContain('secondary');
|
||||
});
|
||||
|
||||
it('strips the model parameter from the JSON schema by default', () => {
|
||||
const host = mockSubagentHost({ spawn: vi.fn() });
|
||||
const tool = agentTool(host);
|
||||
const properties = (tool.parameters as { properties: Record<string, unknown> }).properties;
|
||||
|
||||
expect(properties).not.toHaveProperty('model');
|
||||
expect(properties).toHaveProperty('prompt');
|
||||
});
|
||||
|
||||
it('appends the subagent model description only when provided', () => {
|
||||
const host = mockSubagentHost({ spawn: vi.fn() });
|
||||
const withoutModels = agentTool(host);
|
||||
|
|
|
|||
|
|
@ -458,6 +458,29 @@ describe('current builtin collaboration tools', () => {
|
|||
expect(description.toLowerCase()).toContain('distinct');
|
||||
});
|
||||
|
||||
it('AgentSwarm strips the model parameter from the JSON schema by default', () => {
|
||||
const tool = new AgentSwarmTool(mockSubagentHost({}), mockSwarmMode());
|
||||
const properties = (tool.parameters as { properties: Record<string, unknown> }).properties;
|
||||
|
||||
expect(properties).not.toHaveProperty('model');
|
||||
expect(properties).toHaveProperty('prompt_template');
|
||||
});
|
||||
|
||||
it('AgentSwarm exposes the model parameter in the JSON schema when the experiment is enabled', () => {
|
||||
const tool = new AgentSwarmTool(
|
||||
mockSubagentHost({}),
|
||||
mockSwarmMode(),
|
||||
undefined,
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
const properties = (
|
||||
tool.parameters as { properties: Record<string, { enum?: string[] }> }
|
||||
).properties;
|
||||
|
||||
expect(properties['model']?.enum).toEqual(['primary', 'secondary']);
|
||||
});
|
||||
|
||||
it('AgentSwarm rejects more than 128 subagents at execution time', async () => {
|
||||
const host = mockSubagentHost({ runQueued: vi.fn() });
|
||||
const swarmMode = mockSwarmMode();
|
||||
|
|
|
|||
|
|
@ -4,5 +4,6 @@ export default defineConfig({
|
|||
test: {
|
||||
name: 'kimi-core',
|
||||
include: ['test/**/*.{test,e2e}.ts'],
|
||||
setupFiles: ['test/setup.ts'],
|
||||
},
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue