mirror of https://github.com/openclaw/openclaw.git
fix(gateway): support plugin subagent model overrides
Allow plugin subagent runs to pass provider/model overrides through the typed runtime, gateway validation, and ingress agent path. Apply explicit per-run overrides without persisting them to session state, and cover the gateway + agent behavior with regressions. Regeneration-Prompt: | PR #48277 already forwarded provider and model from plugin subagent runtime calls, but review found two contract gaps: the typed plugin runtime did not declare those fields, and the gateway agent RPC rejected them because the schema and handler path did not accept or propagate them. Make the override path end-to-end valid without broad refactors. Keep the change additive. Extend the plugin runtime and gateway agent request types just enough to carry provider/model overrides. Ensure gateway ingress passes those fields into the existing agent command path, and let agent runs honor explicit per-call overrides without persisting them as session overrides. Add focused regression coverage for the gateway forwarding path and for non-persistent per-run overrides.
This commit is contained in:
parent
76e36720ba
commit
c75f8c607c
|
|
@ -51,6 +51,7 @@ import { applyVerboseOverride } from "../sessions/level-overrides.js";
|
|||
import { applyModelOverrideToSessionEntry } from "../sessions/model-overrides.js";
|
||||
import { resolveSendPolicy } from "../sessions/send-policy.js";
|
||||
import { emitSessionTranscriptUpdate } from "../sessions/transcript-events.js";
|
||||
import { sanitizeForLog } from "../terminal/ansi.js";
|
||||
import { resolveMessageChannel } from "../utils/message-channel.js";
|
||||
import {
|
||||
listAgentIds,
|
||||
|
|
@ -82,6 +83,7 @@ import {
|
|||
modelKey,
|
||||
normalizeModelRef,
|
||||
normalizeProviderId,
|
||||
parseModelRef,
|
||||
resolveConfiguredModelRef,
|
||||
resolveDefaultModelForAgent,
|
||||
resolveThinkingDefault,
|
||||
|
|
@ -124,6 +126,36 @@ const OVERRIDE_FIELDS_CLEARED_BY_DELETE: OverrideFieldClearedByDelete[] = [
|
|||
"claudeCliSessionId",
|
||||
];
|
||||
|
||||
const OVERRIDE_VALUE_MAX_LENGTH = 256;
|
||||
|
||||
function containsControlCharacters(value: string): boolean {
|
||||
for (const char of value) {
|
||||
const code = char.codePointAt(0);
|
||||
if (code === undefined) {
|
||||
continue;
|
||||
}
|
||||
if (code <= 0x1f || (code >= 0x7f && code <= 0x9f)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function normalizeExplicitOverrideInput(raw: string, kind: "provider" | "model"): string {
|
||||
const trimmed = raw.trim();
|
||||
const label = kind === "provider" ? "Provider" : "Model";
|
||||
if (!trimmed) {
|
||||
throw new Error(`${label} override must be non-empty.`);
|
||||
}
|
||||
if (trimmed.length > OVERRIDE_VALUE_MAX_LENGTH) {
|
||||
throw new Error(`${label} override exceeds ${String(OVERRIDE_VALUE_MAX_LENGTH)} characters.`);
|
||||
}
|
||||
if (containsControlCharacters(trimmed)) {
|
||||
throw new Error(`${label} override contains invalid control characters.`);
|
||||
}
|
||||
return trimmed;
|
||||
}
|
||||
|
||||
async function persistSessionEntry(params: PersistSessionEntryParams): Promise<void> {
|
||||
const persisted = await updateSessionStore(params.storePath, (store) => {
|
||||
const merged = mergeSessionEntry(store[params.sessionKey], params.entry);
|
||||
|
|
@ -340,7 +372,7 @@ function runAgentAttempt(params: {
|
|||
resolvedVerboseLevel: VerboseLevel | undefined;
|
||||
agentDir: string;
|
||||
onAgentEvent: (evt: { stream: string; data?: Record<string, unknown> }) => void;
|
||||
primaryProvider: string;
|
||||
authProfileProvider: string;
|
||||
sessionStore?: Record<string, SessionEntry>;
|
||||
storePath?: string;
|
||||
allowTransientCooldownProbe?: boolean;
|
||||
|
|
@ -388,7 +420,7 @@ function runAgentAttempt(params: {
|
|||
params.storePath
|
||||
) {
|
||||
log.warn(
|
||||
`CLI session expired, clearing from session store: provider=${params.providerOverride} sessionKey=${params.sessionKey}`,
|
||||
`CLI session expired, clearing from session store: provider=${sanitizeForLog(params.providerOverride)} sessionKey=${params.sessionKey}`,
|
||||
);
|
||||
|
||||
// Clear the expired session ID from the session store
|
||||
|
|
@ -452,7 +484,7 @@ function runAgentAttempt(params: {
|
|||
}
|
||||
|
||||
const authProfileId =
|
||||
params.providerOverride === params.primaryProvider
|
||||
params.providerOverride === params.authProfileProvider
|
||||
? params.sessionEntry?.authProfileOverride
|
||||
: undefined;
|
||||
return runEmbeddedPiAgent({
|
||||
|
|
@ -937,7 +969,19 @@ async function agentCommandInternal(
|
|||
const hasStoredOverride = Boolean(
|
||||
sessionEntry?.modelOverride || sessionEntry?.providerOverride,
|
||||
);
|
||||
const needsModelCatalog = hasAllowlist || hasStoredOverride;
|
||||
const explicitProviderOverride =
|
||||
typeof opts.provider === "string"
|
||||
? normalizeExplicitOverrideInput(opts.provider, "provider")
|
||||
: undefined;
|
||||
const explicitModelOverride =
|
||||
typeof opts.model === "string"
|
||||
? normalizeExplicitOverrideInput(opts.model, "model")
|
||||
: undefined;
|
||||
const hasExplicitRunOverride = Boolean(explicitProviderOverride || explicitModelOverride);
|
||||
if (hasExplicitRunOverride && opts.allowModelOverride !== true) {
|
||||
throw new Error("Model override is not authorized for this caller.");
|
||||
}
|
||||
const needsModelCatalog = hasAllowlist || hasStoredOverride || hasExplicitRunOverride;
|
||||
let allowedModelKeys = new Set<string>();
|
||||
let allowedModelCatalog: Awaited<ReturnType<typeof loadModelCatalog>> = [];
|
||||
let modelCatalog: Awaited<ReturnType<typeof loadModelCatalog>> | null = null;
|
||||
|
|
@ -1000,13 +1044,38 @@ async function agentCommandInternal(
|
|||
model = normalizedStored.model;
|
||||
}
|
||||
}
|
||||
const providerForAuthProfileValidation = provider;
|
||||
if (hasExplicitRunOverride) {
|
||||
const explicitRef = explicitModelOverride
|
||||
? explicitProviderOverride
|
||||
? normalizeModelRef(explicitProviderOverride, explicitModelOverride)
|
||||
: parseModelRef(explicitModelOverride, provider)
|
||||
: explicitProviderOverride
|
||||
? normalizeModelRef(explicitProviderOverride, model)
|
||||
: null;
|
||||
if (!explicitRef) {
|
||||
throw new Error("Invalid model override.");
|
||||
}
|
||||
const explicitKey = modelKey(explicitRef.provider, explicitRef.model);
|
||||
if (
|
||||
!isCliProvider(explicitRef.provider, cfg) &&
|
||||
!allowAnyModel &&
|
||||
!allowedModelKeys.has(explicitKey)
|
||||
) {
|
||||
throw new Error(
|
||||
`Model override "${sanitizeForLog(explicitRef.provider)}/${sanitizeForLog(explicitRef.model)}" is not allowed for agent "${sessionAgentId}".`,
|
||||
);
|
||||
}
|
||||
provider = explicitRef.provider;
|
||||
model = explicitRef.model;
|
||||
}
|
||||
if (sessionEntry) {
|
||||
const authProfileId = sessionEntry.authProfileOverride;
|
||||
if (authProfileId) {
|
||||
const entry = sessionEntry;
|
||||
const store = ensureAuthProfileStore();
|
||||
const profile = store.profiles[authProfileId];
|
||||
if (!profile || profile.provider !== provider) {
|
||||
if (!profile || profile.provider !== providerForAuthProfileValidation) {
|
||||
if (sessionStore && sessionKey) {
|
||||
await clearSessionAuthProfileOverride({
|
||||
sessionEntry: entry,
|
||||
|
|
@ -1068,6 +1137,7 @@ async function agentCommandInternal(
|
|||
const resolvedSessionFile = await resolveSessionTranscriptFile({
|
||||
sessionId,
|
||||
sessionKey: sessionKey ?? sessionId,
|
||||
storePath,
|
||||
sessionEntry,
|
||||
agentId: sessionAgentId,
|
||||
threadId: opts.threadId,
|
||||
|
|
@ -1132,7 +1202,7 @@ async function agentCommandInternal(
|
|||
skillsSnapshot,
|
||||
resolvedVerboseLevel,
|
||||
agentDir,
|
||||
primaryProvider: provider,
|
||||
authProfileProvider: providerForAuthProfileValidation,
|
||||
sessionStore,
|
||||
storePath,
|
||||
allowTransientCooldownProbe: runOptions?.allowTransientCooldownProbe,
|
||||
|
|
@ -1230,6 +1300,8 @@ export async function agentCommand(
|
|||
// Ingress callers must opt into owner semantics explicitly via
|
||||
// agentCommandFromIngress so network-facing paths cannot inherit this default by accident.
|
||||
senderIsOwner: opts.senderIsOwner ?? true,
|
||||
// Local/CLI callers are trusted by default for per-run model overrides.
|
||||
allowModelOverride: opts.allowModelOverride ?? true,
|
||||
},
|
||||
runtime,
|
||||
deps,
|
||||
|
|
@ -1246,10 +1318,14 @@ export async function agentCommandFromIngress(
|
|||
// This keeps network-facing callers from silently picking up the local trusted default.
|
||||
throw new Error("senderIsOwner must be explicitly set for ingress agent runs.");
|
||||
}
|
||||
if (typeof opts.allowModelOverride !== "boolean") {
|
||||
throw new Error("allowModelOverride must be explicitly set for ingress agent runs.");
|
||||
}
|
||||
return await agentCommandInternal(
|
||||
{
|
||||
...opts,
|
||||
senderIsOwner: opts.senderIsOwner,
|
||||
allowModelOverride: opts.allowModelOverride,
|
||||
},
|
||||
runtime,
|
||||
deps,
|
||||
|
|
|
|||
|
|
@ -39,6 +39,10 @@ export type AgentCommandOpts = {
|
|||
clientTools?: ClientToolDefinition[];
|
||||
/** Agent id override (must exist in config). */
|
||||
agentId?: string;
|
||||
/** Per-run provider override. */
|
||||
provider?: string;
|
||||
/** Per-run model override. */
|
||||
model?: string;
|
||||
to?: string;
|
||||
sessionId?: string;
|
||||
sessionKey?: string;
|
||||
|
|
@ -65,6 +69,8 @@ export type AgentCommandOpts = {
|
|||
runContext?: AgentRunContext;
|
||||
/** Whether this caller is authorized for owner-only tools (defaults true for local CLI calls). */
|
||||
senderIsOwner?: boolean;
|
||||
/** Whether this caller is authorized to use provider/model per-run overrides. */
|
||||
allowModelOverride?: boolean;
|
||||
/** Group/spawn metadata for subagent policy inheritance and routing context. */
|
||||
groupId?: SpawnedRunMetadata["groupId"];
|
||||
groupChannel?: SpawnedRunMetadata["groupChannel"];
|
||||
|
|
@ -84,7 +90,12 @@ export type AgentCommandOpts = {
|
|||
workspaceDir?: SpawnedRunMetadata["workspaceDir"];
|
||||
};
|
||||
|
||||
export type AgentCommandIngressOpts = Omit<AgentCommandOpts, "senderIsOwner"> & {
|
||||
/** Ingress callsites must always pass explicit owner authorization state. */
|
||||
export type AgentCommandIngressOpts = Omit<
|
||||
AgentCommandOpts,
|
||||
"senderIsOwner" | "allowModelOverride"
|
||||
> & {
|
||||
/** Ingress callsites must always pass explicit owner-tool authorization state. */
|
||||
senderIsOwner: boolean;
|
||||
/** Ingress callsites must always pass explicit model-override authorization state. */
|
||||
allowModelOverride: boolean;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -686,6 +686,32 @@ describe("agentCommand", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("applies per-run provider and model overrides without persisting them", async () => {
|
||||
await withTempHome(async (home) => {
|
||||
const store = path.join(home, "sessions.json");
|
||||
mockConfig(home, store);
|
||||
|
||||
await agentCommand(
|
||||
{
|
||||
message: "use the override",
|
||||
sessionKey: "agent:main:subagent:run-override",
|
||||
provider: "openai",
|
||||
model: "gpt-4.1-mini",
|
||||
},
|
||||
runtime,
|
||||
);
|
||||
|
||||
expectLastRunProviderModel("openai", "gpt-4.1-mini");
|
||||
|
||||
const saved = readSessionStore<{
|
||||
providerOverride?: string;
|
||||
modelOverride?: string;
|
||||
}>(store);
|
||||
expect(saved["agent:main:subagent:run-override"]?.providerOverride).toBeUndefined();
|
||||
expect(saved["agent:main:subagent:run-override"]?.modelOverride).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps explicit sessionKey even when sessionId exists elsewhere", async () => {
|
||||
await withTempHome(async (home) => {
|
||||
const store = path.join(home, "sessions.json");
|
||||
|
|
|
|||
|
|
@ -75,6 +75,8 @@ export const AgentParamsSchema = Type.Object(
|
|||
{
|
||||
message: NonEmptyString,
|
||||
agentId: Type.Optional(NonEmptyString),
|
||||
provider: Type.Optional(Type.String()),
|
||||
model: Type.Optional(Type.String()),
|
||||
to: Type.Optional(Type.String()),
|
||||
replyTo: Type.Optional(Type.String()),
|
||||
sessionId: Type.Optional(Type.String()),
|
||||
|
|
|
|||
|
|
@ -303,6 +303,30 @@ describe("gateway agent handler", () => {
|
|||
expect(capturedEntry?.acp).toEqual(existingAcpMeta);
|
||||
});
|
||||
|
||||
it("forwards provider and model overrides to ingress agent runs", async () => {
|
||||
primeMainAgentRun();
|
||||
|
||||
await invokeAgent(
|
||||
{
|
||||
message: "test override",
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
provider: "anthropic",
|
||||
model: "claude-haiku-4-6",
|
||||
idempotencyKey: "test-idem-model-override",
|
||||
},
|
||||
{ reqId: "test-idem-model-override" },
|
||||
);
|
||||
|
||||
const lastCall = mocks.agentCommand.mock.calls.at(-1);
|
||||
expect(lastCall?.[0]).toEqual(
|
||||
expect.objectContaining({
|
||||
provider: "anthropic",
|
||||
model: "claude-haiku-4-6",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves cliSessionIds from existing session entry", async () => {
|
||||
const existingCliSessionIds = { "claude-cli": "abc-123-def" };
|
||||
const existingClaudeCliSessionId = "abc-123-def";
|
||||
|
|
|
|||
|
|
@ -162,6 +162,8 @@ export const agentHandlers: GatewayRequestHandlers = {
|
|||
const request = p as {
|
||||
message: string;
|
||||
agentId?: string;
|
||||
provider?: string;
|
||||
model?: string;
|
||||
to?: string;
|
||||
replyTo?: string;
|
||||
sessionId?: string;
|
||||
|
|
@ -584,6 +586,8 @@ export const agentHandlers: GatewayRequestHandlers = {
|
|||
ingressOpts: {
|
||||
message,
|
||||
images,
|
||||
provider: request.provider,
|
||||
model: request.model,
|
||||
to: resolvedTo,
|
||||
sessionId: resolvedSessionId,
|
||||
sessionKey: resolvedSessionKey,
|
||||
|
|
|
|||
|
|
@ -8,6 +8,8 @@ export type { RuntimeLogger };
|
|||
export type SubagentRunParams = {
|
||||
sessionKey: string;
|
||||
message: string;
|
||||
provider?: string;
|
||||
model?: string;
|
||||
extraSystemPrompt?: string;
|
||||
lane?: string;
|
||||
deliver?: boolean;
|
||||
|
|
|
|||
Loading…
Reference in New Issue