fix(agents): reject unusable explicit spawn models (#125931)

Prevent explicit subagent model requests from silently falling back by validating model policy and provider ownership before child state is created.

Co-authored-by: Heming Zeng <hermanzeng@foxmail.com>
Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
Heming Zeng
2026-08-20 15:49:53 +08:00
committed by GitHub
parent 531fc9b365
commit 366dbebd98
5 changed files with 318 additions and 34 deletions
@@ -4,7 +4,12 @@ import { isIncognitoSessionKey } from "../../../routing/session-key.js";
import { resolveUserPath } from "../../../utils.js";
import { resolveAgentDir } from "../../agent-scope-config.js";
import { findModelCatalogEntry } from "../../model-catalog-lookup.js";
import { resolveDefaultModelForAgent } from "../../model-selection.js";
import type { ModelCatalogEntry } from "../../model-catalog.types.js";
import {
findNormalizedProviderValue,
resolveAllowedModelRef,
resolveDefaultModelForAgent,
} from "../../model-selection.js";
import { supportsModelTools } from "../../model-tool-support.js";
import { summarizeSpawnError } from "../../spawn-pipeline.js";
import { resolveSpawnSandboxError, mintSpawnSessionKey } from "../../spawn-plan.js";
@@ -26,7 +31,6 @@ import {
readRequesterThinkingLevel,
} from "./subagent-spawn-requester-prefs.js";
import {
loadPreparedModelCatalog,
normalizeDeliveryContext,
resolveAgentConfig,
resolveSandboxRuntimeStatus,
@@ -47,24 +51,23 @@ function buildResolvedSubagentModelMetadata(resolvedModel?: string): {
};
}
async function resolveCollectorOutputModelError(params: {
async function resolveSpawnModelError(params: {
cfg: OpenClawConfig;
targetAgentId: string;
targetAgentDir: string;
workspaceDir?: string;
request: SpawnSubagentParams;
resolvedModel?: string;
}): Promise<string | undefined> {
const selected = splitModelRef(params.resolvedModel);
const fallback = resolveDefaultModelForAgent({
cfg: params.cfg,
agentId: params.targetAgentId,
});
const provider = selected.provider ?? fallback.provider;
const model = selected.model ?? fallback.model;
if (!provider || !model) {
const { cfg, targetAgentId } = params;
const requestedModel = normalizeOptionalString(params.request.model);
if (!requestedModel && !params.request.outputSchema) {
return undefined;
}
let catalog: Awaited<ReturnType<typeof loadPreparedModelCatalog>>;
const defaults = resolveDefaultModelForAgent({ cfg, agentId: targetAgentId });
const selected = splitModelRef(params.resolvedModel);
const provider = selected.provider ?? defaults.provider;
let catalog: ModelCatalogEntry[];
try {
catalog = await getSubagentSpawnDeps().loadPreparedModelCatalog({
config: params.cfg,
@@ -75,13 +78,53 @@ async function resolveCollectorOutputModelError(params: {
scopedLiveProviderDiscovery: true,
});
} catch (error) {
return `sessions_spawn could not verify outputSchema model capabilities: ${summarizeSpawnError(error)}`;
return `sessions_spawn could not verify ${requestedModel ? "the requested model" : "outputSchema model capabilities"}: ${summarizeSpawnError(error)}`;
}
const entry = findModelCatalogEntry(catalog, { provider, modelId: model });
if (!entry || supportsModelTools(entry)) {
return undefined;
if (!requestedModel) {
const model = selected.model ?? defaults.model;
const entry = model && findModelCatalogEntry(catalog, { provider, modelId: model });
return entry && !supportsModelTools(entry)
? `sessions_spawn outputSchema requires a tool-capable target model; "${provider}/${model}" declares compat.supportsTools=false.`
: undefined;
}
return `sessions_spawn outputSchema requires a tool-capable target model; "${provider}/${model}" declares compat.supportsTools=false.`;
const selection = {
cfg,
catalog,
defaultProvider: defaults.provider,
defaultModel: defaults.model,
agentId: targetAgentId,
};
const resolved = resolveAllowedModelRef({
...selection,
raw: requestedModel,
});
if ("error" in resolved) {
return `sessions_spawn model "${requestedModel}" is not usable: ${resolved.error}`;
}
const entry = findModelCatalogEntry(catalog, {
provider: resolved.ref.provider,
modelId: resolved.ref.model,
});
if (!entry) {
const resolvedProvider = resolved.ref.provider;
const knownProvider =
findNormalizedProviderValue(cfg.models?.providers, resolvedProvider) ||
catalog.some((catalogEntry) => catalogEntry.provider === resolvedProvider) ||
getSubagentSpawnDeps().resolveProviderRefOwnership({
provider: resolvedProvider,
config: cfg,
workspaceDir: params.workspaceDir,
}).status === "owned";
if (!knownProvider) {
return `sessions_spawn model "${requestedModel}" is not usable: unknown model provider "${resolvedProvider}"`;
}
}
if (params.request.outputSchema && entry && !supportsModelTools(entry)) {
return `sessions_spawn outputSchema requires a tool-capable target model; "${resolved.ref.provider}/${resolved.ref.model}" declares compat.supportsTools=false.`;
}
return undefined;
}
type ResolvedSubagentChildPlan = {
@@ -221,6 +264,24 @@ export async function resolveSubagentChildPlan(params: {
};
}
const { resolvedModel } = modelPlan;
const modelError = await resolveSpawnModelError({
cfg: params.cfg,
targetAgentId: params.targetAgentId,
targetAgentDir,
workspaceDir: spawnedWorkspaceDir,
request: params.request,
resolvedModel,
});
if (modelError) {
return {
ok: false,
result: {
status: "error",
error: modelError,
...(params.request.outputSchema ? { childSessionKey } : {}),
},
};
}
const resolvedLaunchModel = splitModelRef(resolvedModel);
const launchAuthorization: SubagentLaunchAuthorization | undefined =
params.request.model?.trim() && resolvedLaunchModel.model
@@ -231,21 +292,6 @@ export async function resolveSubagentChildPlan(params: {
},
}
: undefined;
if (params.request.outputSchema) {
const outputModelError = await resolveCollectorOutputModelError({
cfg: params.cfg,
targetAgentId: params.targetAgentId,
targetAgentDir,
workspaceDir: spawnedWorkspaceDir,
resolvedModel,
});
if (outputModelError) {
return {
ok: false,
result: { status: "error", error: outputModelError, childSessionKey },
};
}
}
return {
ok: true,
resolved: {
@@ -8,6 +8,7 @@ import {
getRuntimeConfig,
hasInProcessGatewayContext,
loadPreparedModelCatalog,
resolveProviderRefOwnership,
resolveContextEngine,
} from "./subagent-spawn.runtime.js";
@@ -20,6 +21,7 @@ type SubagentSpawnDeps = {
hasInProcessGatewayContext: typeof hasInProcessGatewayContext;
ensureContextEnginesInitialized: typeof ensureContextEnginesInitialized;
loadPreparedModelCatalog: typeof loadPreparedModelCatalog;
resolveProviderRefOwnership: typeof resolveProviderRefOwnership;
resolveContextEngine: typeof resolveContextEngine;
};
@@ -32,6 +34,7 @@ const defaultSubagentSpawnDeps: SubagentSpawnDeps = {
hasInProcessGatewayContext,
ensureContextEnginesInitialized,
loadPreparedModelCatalog,
resolveProviderRefOwnership,
resolveContextEngine,
};
@@ -23,6 +23,7 @@ export {
export { getSessionBindingService } from "../../../infra/outbound/session-binding-service.js";
export { resolveGatewaySessionStoreTarget } from "../../../gateway/session-utils.js";
export { getGlobalHookRunner } from "../../../plugins/hook-runner-global.js";
export { resolveProviderRefOwnership } from "../../../plugins/providers.js";
export { emitSessionLifecycleEvent } from "../../../sessions/session-lifecycle-events.js";
export {
mergeDeliveryContext,
@@ -134,6 +134,7 @@ export async function loadSubagentSpawnModuleForTest(params: {
getRuntimeConfig?: () => Record<string, unknown>;
loadSessionStoreMock?: MockFn;
loadPreparedModelCatalogMock?: MockFn;
resolveProviderRefOwnershipMock?: MockFn;
ensureContextEnginesInitializedMock?: MockFn;
updateSessionStoreMock?: MockFn;
forkSessionEntryFromParentMock?: MockFn;
@@ -268,6 +269,11 @@ export async function loadSubagentSpawnModuleForTest(params: {
createSubagentSpawnTestConfig(params.workspaceDir ?? os.tmpdir()),
loadPreparedModelCatalog: (...args: unknown[]) =>
params.loadPreparedModelCatalogMock?.(...args) ?? [],
resolveProviderRefOwnership: (...args: unknown[]) =>
params.resolveProviderRefOwnershipMock?.(...args) ?? {
status: "owned",
pluginIds: ["test-provider"],
},
loadSessionEntry: (scope: { storePath?: string; sessionKey: string }) =>
((params.loadSessionStoreMock?.(scope.storePath) ?? {}) as SessionStore)[scope.sessionKey],
loadSessionStore: params.loadSessionStoreMock ?? (() => ({})),
@@ -5,6 +5,7 @@ import { createRequireRecord } from "openclaw/plugin-sdk/test-fixtures";
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../../../config/types.openclaw.js";
import { resolveIncognitoOpenClawAgentSqlitePath } from "../../../state/openclaw-agent-db.paths.js";
import { resolveUserPath } from "../../../utils.js";
import { installAcceptedSubagentGatewayMock } from "../../test-helpers/subagent-gateway.js";
import { testing as swarmSchedulerTesting } from "../swarm/swarm-scheduler.test-support.js";
import {
@@ -21,6 +22,7 @@ const hoisted = vi.hoisted(() => ({
throw new Error("full model catalog should not materialize");
}),
loadPreparedModelCatalogMock: vi.fn(),
resolveProviderRefOwnershipMock: vi.fn(),
updateSessionStoreMock: vi.fn(),
registerSubagentRunMock: vi.fn(),
startQueuedSubagentRunMock: vi.fn(),
@@ -73,6 +75,13 @@ function firstRegisteredSubagentRun(): Record<string, unknown> {
return requireRecord(hoisted.registerSubagentRunMock.mock.calls[0]?.[0]);
}
function expectNoChildSpawnSideEffects(): void {
expect(hoisted.updateSessionStoreMock).not.toHaveBeenCalled();
expect(hoisted.registerSubagentRunMock).not.toHaveBeenCalled();
expect(hoisted.callGatewayMock).not.toHaveBeenCalled();
expect(hoisted.emitSessionLifecycleEventMock).not.toHaveBeenCalled();
}
type InheritedSpawnPreferenceCase = {
name: string;
task: string;
@@ -164,6 +173,7 @@ describe("spawnSubagentDirect seam flow", () => {
getRuntimeConfig: () => hoisted.configOverride,
loadSessionStoreMock: hoisted.loadSessionStoreMock,
loadPreparedModelCatalogMock: hoisted.loadPreparedModelCatalogMock,
resolveProviderRefOwnershipMock: hoisted.resolveProviderRefOwnershipMock,
updateSessionStoreMock: hoisted.updateSessionStoreMock,
registerSubagentRunMock: hoisted.registerSubagentRunMock,
startQueuedSubagentRunMock: hoisted.startQueuedSubagentRunMock,
@@ -187,6 +197,10 @@ describe("spawnSubagentDirect seam flow", () => {
hoisted.loadSessionStoreMock.mockReset();
hoisted.loadFullModelCatalogMock.mockClear();
hoisted.loadPreparedModelCatalogMock.mockReset().mockResolvedValue([]);
hoisted.resolveProviderRefOwnershipMock.mockReset().mockReturnValue({
status: "owned",
pluginIds: ["test-provider"],
});
hoisted.updateSessionStoreMock.mockReset();
hoisted.registerSubagentRunMock.mockReset();
hoisted.startQueuedSubagentRunMock.mockReset().mockReturnValue(true);
@@ -515,6 +529,219 @@ describe("spawnSubagentDirect seam flow", () => {
});
});
it("rejects an explicit non-allowlisted model before creating child state", async () => {
hoisted.configOverride = createConfigOverride({
agents: {
defaults: {
workspace: os.tmpdir(),
modelPolicy: { allow: ["openai/gpt-5.4"] },
},
list: [{ id: "main", workspace: "/tmp/workspace-main" }],
},
});
hoisted.loadPreparedModelCatalogMock.mockResolvedValue([
{ provider: "openai", id: "gpt-5.4", name: "GPT-5.4" },
{
provider: "anthropic",
id: "claude-sonnet-4-6",
name: "Claude Sonnet 4.6",
},
]);
const result = await spawnSubagentDirect(
{ task: "must honor model policy", model: "anthropic/claude-sonnet-4-6" },
{ agentSessionKey: "agent:main:main" },
);
expect(result.status).toBe("error");
expect(result.error).toContain("model not allowed: anthropic/claude-sonnet-4-6");
expectNoChildSpawnSideEffects();
});
it("rejects an unknown-provider model under unrestricted policy before creating child state", async () => {
hoisted.resolveProviderRefOwnershipMock.mockReturnValue({ status: "unowned" });
hoisted.loadPreparedModelCatalogMock.mockResolvedValue([
{ provider: "openai", id: "gpt-5.4", name: "GPT-5.4" },
]);
const result = await spawnSubagentDirect(
{ task: "do not substitute an unknown provider", model: "unknown-provider/gpt-5.4" },
{ agentSessionKey: "agent:main:main" },
);
expect(result.status).toBe("error");
expect(result.error).toContain('unknown model provider "unknown-provider"');
expectNoChildSpawnSideEffects();
});
it("does not treat ambiguous provider ownership as runnable", async () => {
hoisted.resolveProviderRefOwnershipMock.mockReturnValue({
status: "ambiguous",
pluginIds: ["provider-a", "provider-b"],
});
const result = await spawnSubagentDirect(
{ task: "do not guess an owner", model: "ambiguous-provider/gpt-5.4" },
{ agentSessionKey: "agent:main:main" },
);
expect(result.status).toBe("error");
expect(result.error).toContain('unknown model provider "ambiguous-provider"');
expectNoChildSpawnSideEffects();
});
it("accepts a catalog-missing model from a known provider under unrestricted policy", async () => {
hoisted.loadPreparedModelCatalogMock.mockResolvedValue([
{ provider: "openai", id: "gpt-5.4", name: "GPT-5.4" },
]);
const result = await spawnSubagentDirect(
{ task: "use a newly released model", model: "openai/gpt-new" },
{ agentSessionKey: "agent:main:main" },
);
expect(result).toMatchObject({
status: "accepted",
modelApplied: true,
resolvedModel: "openai/gpt-new",
resolvedProvider: "openai",
});
expect(hoisted.resolveProviderRefOwnershipMock).not.toHaveBeenCalled();
});
it("accepts a catalog-missing model known only through provider ownership", async () => {
hoisted.loadPreparedModelCatalogMock.mockResolvedValue([]);
const result = await spawnSubagentDirect(
{ task: "use a plugin-owned provider", model: "plugin-provider/new-model" },
{ agentSessionKey: "agent:main:main" },
);
expect(result).toMatchObject({
status: "accepted",
resolvedModel: "plugin-provider/new-model",
resolvedProvider: "plugin-provider",
});
expect(hoisted.resolveProviderRefOwnershipMock).toHaveBeenCalledWith({
provider: "plugin-provider",
config: hoisted.configOverride,
workspaceDir: resolveUserPath("/tmp/workspace-main"),
});
});
it("accepts a catalog-missing model from a configured custom provider", async () => {
hoisted.configOverride = createConfigOverride({
models: {
providers: {
loopback: {
api: "openai-completions",
baseUrl: "http://127.0.0.1:43123/v1",
models: [],
},
},
},
});
hoisted.resolveProviderRefOwnershipMock.mockReturnValue({ status: "unowned" });
const result = await spawnSubagentDirect(
{ task: "use the configured loopback provider", model: "loopback/new-model" },
{ agentSessionKey: "agent:main:main" },
);
expect(result).toMatchObject({
status: "accepted",
modelApplied: true,
resolvedModel: "loopback/new-model",
resolvedProvider: "loopback",
});
});
it.each([
{ policy: "exact", allow: ["future-provider/new-model"] },
{ policy: "provider wildcard", allow: ["future-provider/*"] },
])("rejects an unowned catalog-missing ref under a strict $policy policy", async ({ allow }) => {
hoisted.configOverride = createConfigOverride({
agents: {
defaults: {
workspace: os.tmpdir(),
modelPolicy: { allow },
},
list: [{ id: "main", workspace: "/tmp/workspace-main" }],
},
});
hoisted.resolveProviderRefOwnershipMock.mockReturnValue({ status: "unowned" });
const result = await spawnSubagentDirect(
{ task: "do not launch an unowned provider", model: "future-provider/new-model" },
{ agentSessionKey: "agent:main:main" },
);
expect(result.status).toBe("error");
expect(result.error).toContain('unknown model provider "future-provider"');
expect(hoisted.resolveProviderRefOwnershipMock).toHaveBeenCalledWith({
provider: "future-provider",
config: hoisted.configOverride,
workspaceDir: resolveUserPath("/tmp/workspace-main"),
});
expectNoChildSpawnSideEffects();
});
it.each([
{
name: "alias",
model: "fast",
models: { "openai/gpt-5.4": { alias: "fast" } },
},
{
name: "bare model ref",
model: "gpt-5.4",
models: { "openai/gpt-5.4": {} },
},
])("validates an explicit $name through the target policy", async ({ model, models }) => {
hoisted.configOverride = createConfigOverride({
agents: {
defaults: { workspace: os.tmpdir(), models },
list: [{ id: "main", workspace: "/tmp/workspace-main" }],
},
});
hoisted.loadPreparedModelCatalogMock.mockResolvedValue([
{ provider: "openai", id: "gpt-5.4", name: "GPT-5.4" },
]);
const result = await spawnSubagentDirect(
{ task: `use ${model}`, model },
{ agentSessionKey: "agent:main:main" },
);
expect(result).toMatchObject({ status: "accepted", modelApplied: true });
});
it("does not load the model catalog for an implicit default", async () => {
const result = await spawnSubagentDirect(
{ task: "inherit the default model" },
{ agentSessionKey: "agent:main:main" },
);
expect(result.status).toBe("accepted");
expect(hoisted.loadPreparedModelCatalogMock).not.toHaveBeenCalled();
expect(hoisted.resolveProviderRefOwnershipMock).not.toHaveBeenCalled();
});
it("rejects an explicit model when catalog validation fails without creating child state", async () => {
hoisted.loadPreparedModelCatalogMock.mockRejectedValue(new Error("catalog unavailable"));
const result = await spawnSubagentDirect(
{ task: "validate before launch", model: "openai/gpt-5.4" },
{ agentSessionKey: "agent:main:main" },
);
expect(result.status).toBe("error");
expect(result.error).toContain(
"sessions_spawn could not verify the requested model: catalog unavailable",
);
expectNoChildSpawnSideEffects();
});
it("aborts a collector cancelled while its gateway launch is in flight", async () => {
hoisted.configOverride = createConfigOverride({ tools: { swarm: true } });
hoisted.startQueuedSubagentRunMock.mockReturnValue(false);
@@ -1062,10 +1289,11 @@ describe("spawnSubagentDirect seam flow", () => {
expect(rejected.status).toBe("error");
expect(rejected.error).toContain("requires a tool-capable target model");
expect(hoisted.loadFullModelCatalogMock).not.toHaveBeenCalled();
expect(hoisted.loadPreparedModelCatalogMock).toHaveBeenCalledTimes(1);
expect(hoisted.loadPreparedModelCatalogMock).toHaveBeenCalledWith({
config: hoisted.configOverride,
agentDir: expect.any(String),
workspaceDir: "/tmp/workspace-main",
workspaceDir: resolveUserPath("/tmp/workspace-main"),
readOnly: true,
providerDiscoveryProviderIds: ["openai"],
scopedLiveProviderDiscovery: true,
@@ -1710,7 +1938,7 @@ describe("spawnSubagentDirect seam flow", () => {
const childSessionKey = result.childSessionKey as string;
const childEntry = persistedStore?.[childSessionKey];
expect(childEntry?.spawnedWorkspaceDir).toBe("/tmp/requester-workspace");
expect(childEntry?.spawnedCwd).toBe("/tmp/task-repo");
expect(childEntry?.spawnedCwd).toBe(resolveUserPath("/tmp/task-repo"));
const agentRequest = gatewayRequest("agent");
const agentParams = requireRecord(agentRequest.params);