ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-28 13:26:04 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(cron): warn when allowed web_search has no provider

Browse Source
This commit is contained in:
LiLan0125
2026-06-30 01:36:22 +08:00
committed by GitHub
parent 597a0ba43c
commit 685b95b607
5 changed files with 325 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/cron/isolated-agent/run.test-harness.ts
+11
View File
@@ -80,6 +80,8 @@ export const getChannelPluginMock = createMock();
export const retireSessionMcpRuntimeMock = createMock();
export const callGatewayMock = createMock();
export const ensureRuntimePluginsLoadedMock = createMock();
export const listWebSearchProvidersMock = createMock();
export const resolveWebSearchProviderIdMock = createMock();
const resolveBootstrapWarningSignaturesSeenMock = createMock();
const resolveCronStyleNowMock = createMock();
@@ -161,6 +163,11 @@ vi.mock("../../plugins/runtime-plugins.runtime.js", () => ({
ensureRuntimePluginsLoaded: ensureRuntimePluginsLoadedMock,
}));
vi.mock("../../web-search/runtime.js", () => ({
listWebSearchProviders: listWebSearchProvidersMock,
resolveWebSearchProviderId: resolveWebSearchProviderIdMock,
}));
vi.mock("../../skills/runtime/cron-snapshot.runtime.js", () => ({
canExecRequestNode: vi.fn(() => false),
getRemoteSkillEligibility: getRemoteSkillEligibilityMock,
@@ -673,6 +680,10 @@ export function resetRunCronIsolatedAgentTurnHarness(): void {
setSessionRuntimeModelMock.mockReturnValue(undefined);
logWarnMock.mockReset();
ensureRuntimePluginsLoadedMock.mockReset();
listWebSearchProvidersMock.mockReset();
listWebSearchProvidersMock.mockReturnValue([{ id: "duckduckgo" }]);
resolveWebSearchProviderIdMock.mockReset();
resolveWebSearchProviderIdMock.mockReturnValue("duckduckgo");
}
export function clearFastTestEnv(): string | undefined {
src/cron/isolated-agent/run.tools-allow.test.ts
+132
View File
@@ -1,10 +1,15 @@
// Tool allowlist tests cover tool availability for isolated cron runs.
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE } from "../run-diagnostics.js";
import "../../agents/test-helpers/fast-coding-tools.js";
import {
listWebSearchProvidersMock,
loadModelCatalogMock,
loadRunCronIsolatedAgentTurn,
resolveConfiguredModelRefMock,
resetRunCronIsolatedAgentTurnHarness,
resolveDeliveryTargetMock,
resolveWebSearchProviderIdMock,
runEmbeddedAgentMock,
runWithModelFallbackMock,
} from "./run.test-harness.js";
@@ -46,6 +51,23 @@ function makeParamsWithToolsAllow(toolsAllow: string[]) {
};
}
function makeParamsWithDefaultToolsAllow(toolsAllow: string[]) {
const params = makeParams();
const job = params.job as Record<string, unknown>;
return {
...params,
job: {
...job,
payload: {
kind: "agentTurn",
message: "check allowed tools",
toolsAllow,
toolsAllowIsDefault: true,
},
} as never,
};
}
function requireEmbeddedAgentCall(): {
jobId?: string;
toolsAllow?: string[];
@@ -127,4 +149,114 @@ describe("runCronIsolatedAgentTurn toolsAllow passthrough", () => {
expect(call.toolsAllow).toEqual(["maniple__check_idle_workers"]);
},
);
it(
"adds cron diagnostics when web_search is allowed without a selected provider",
{ timeout: RUN_TOOLS_ALLOW_TIMEOUT_MS },
async () => {
listWebSearchProvidersMock.mockReturnValue([{ id: "duckduckgo" }]);
resolveWebSearchProviderIdMock.mockReturnValue("");
const result = await runCronIsolatedAgentTurn(makeParamsWithToolsAllow(["web_search"]));
expect(result.status).toBe("ok");
expect(runEmbeddedAgentMock).toHaveBeenCalledTimes(1);
const call = requireEmbeddedAgentCall();
expect(call.toolsAllow).toEqual(["web_search"]);
expect(result.diagnostics?.summary).toBe(MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE);
expect(result.diagnostics?.entries).toEqual([
{
ts: expect.any(Number),
source: "cron-preflight",
severity: "warn",
message: MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE,
toolName: "web_search",
},
]);
},
);
it(
"does not warn for default-derived toolsAllow that includes web_search",
{ timeout: RUN_TOOLS_ALLOW_TIMEOUT_MS },
async () => {
listWebSearchProvidersMock.mockReturnValue([]);
const result = await runCronIsolatedAgentTurn(
makeParamsWithDefaultToolsAllow(["web_search"]),
);
expect(result.status).toBe("ok");
expect(result.diagnostics).toBeUndefined();
},
);
it(
"does not warn when native web_search suppresses the managed provider tool",
{ timeout: RUN_TOOLS_ALLOW_TIMEOUT_MS },
async () => {
listWebSearchProvidersMock.mockReturnValue([]);
resolveConfiguredModelRefMock.mockReturnValue({
provider: "gateway",
model: "gpt-5.5",
});
loadModelCatalogMock.mockResolvedValue([
{
id: "gpt-5.5",
name: "GPT-5.5",
provider: "gateway",
api: "openai-chatgpt-responses",
},
]);
const result = await runCronIsolatedAgentTurn({
...makeParamsWithToolsAllow(["web_search"]),
cfg: {
tools: {
web: {
search: {
enabled: true,
openaiCodex: {
enabled: true,
mode: "cached",
},
},
},
},
},
});
expect(result.status).toBe("ok");
expect(result.diagnostics).toBeUndefined();
},
);
it(
"keeps web_search provider diagnostics when the run aborts",
{ timeout: RUN_TOOLS_ALLOW_TIMEOUT_MS },
async () => {
listWebSearchProvidersMock.mockReturnValue([]);
resolveWebSearchProviderIdMock.mockReturnValue("");
runWithModelFallbackMock.mockResolvedValueOnce({
result: {
payloads: [],
meta: {
aborted: true,
agentMeta: {},
},
},
provider: "openai",
model: "gpt-5.4",
attempts: [],
});
const result = await runCronIsolatedAgentTurn(makeParamsWithToolsAllow(["web_search"]));
expect(result.status).toBe("error");
expect(result.diagnostics?.entries.map((entry) => entry.message)).toEqual([
MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE,
"cron isolated agent run aborted",
]);
},
);
});
src/cron/isolated-agent/run.ts
+99 -6
View File
@@ -3,6 +3,7 @@ import { normalizeOptionalString } from "@openclaw/normalization-core/string-coe
import { retireSessionMcpRuntime } from "../../agents/agent-bundle-mcp-tools.js";
import { hasAnyAuthProfileStoreSource } from "../../agents/auth-profiles/source-check.js";
import { resolveAgentHarnessPolicy } from "../../agents/harness/policy.js";
import { findModelInCatalog } from "../../agents/model-catalog-lookup.js";
import { listOpenAIAuthProfileProvidersForAgentRuntime } from "../../agents/openai-routing.js";
import { expandToolGroups, normalizeToolName } from "../../agents/tool-policy.js";
import { deriveContextPromptTokens } from "../../agents/usage.js";
@@ -47,9 +48,11 @@ import {
type CronDeliveryPlan,
} from "../delivery-plan.js";
import {
createCronRunDiagnosticsFromMissingWebSearchProvider,
createCronRunDiagnosticsFromAgentResult,
createCronRunDiagnosticsFromError,
mergeCronRunDiagnostics,
toolsAllowRequestsWebSearch,
} from "../run-diagnostics.js";
import { resolveCronAbortReasonText } from "../service/execution-errors.js";
import { resolveCronDeliverySessionKey } from "../session-target.js";
@@ -60,6 +63,7 @@ import type {
CronDeliveryTraceMessageTarget,
CronDeliveryTraceTarget,
CronJob,
CronRunDiagnostics,
CronRunTelemetry,
} from "../types.js";
import { resolveCronChannelOutputPolicy } from "./channel-output-policy.js";
@@ -132,6 +136,10 @@ const cronModelPreflightRuntimeLoader = createLazyImportLoader(
const runtimePluginsLoader = createLazyImportLoader(
() => import("../../plugins/runtime-plugins.runtime.js"),
);
const codexNativeWebSearchLoader = createLazyImportLoader(
() => import("../../agents/codex-native-web-search.js"),
);
const webSearchRuntimeLoader = createLazyImportLoader(() => import("../../web-search/runtime.js"));
async function loadSessionStoreRuntime() {
return await sessionStoreRuntimeLoader.load();
@@ -169,6 +177,14 @@ async function loadRuntimePlugins() {
return await runtimePluginsLoader.load();
}
async function loadCodexNativeWebSearch() {
return await codexNativeWebSearchLoader.load();
}
async function loadWebSearchRuntime() {
return await webSearchRuntimeLoader.load();
}
function hasConfiguredAuthProfiles(cfg: OpenClawConfig): boolean {
return (
Boolean(cfg.auth?.profiles && Object.keys(cfg.auth.profiles).length > 0) ||
@@ -326,6 +342,59 @@ function canPromptForMessageTool(params: {
);
}
async function createCronToolsAllowPreflightDiagnostics(params: {
cfg: OpenClawConfig;
jobId: string;
provider: string;
model: string;
modelApi?: string;
agentId?: string;
agentDir?: string;
sessionKey?: string;
agentPayload: Extract<CronJob["payload"], { kind: "agentTurn" }> | null;
}): Promise<CronRunDiagnostics | undefined> {
const toolsAllow = params.agentPayload?.toolsAllow;
if (
params.agentPayload?.toolsAllowIsDefault === true ||
!toolsAllowRequestsWebSearch(toolsAllow)
) {
return undefined;
}
try {
const { shouldSuppressManagedWebSearchTool } = await loadCodexNativeWebSearch();
if (
shouldSuppressManagedWebSearchTool({
config: params.cfg,
modelProvider: params.provider,
modelApi: params.modelApi,
modelId: params.model,
agentId: params.agentId,
sessionKey: params.sessionKey,
agentDir: params.agentDir,
})
) {
return undefined;
}
const { listWebSearchProviders, resolveWebSearchProviderId } = await loadWebSearchRuntime();
const webSearchProviders = listWebSearchProviders({ config: params.cfg });
return createCronRunDiagnosticsFromMissingWebSearchProvider({
toolsAllow,
hasWebSearchProvider: Boolean(
resolveWebSearchProviderId({
config: params.cfg,
agentDir: params.agentDir,
providers: webSearchProviders,
}),
),
});
} catch (error) {
logWarn(
`[cron:${params.jobId}] Failed to inspect web_search providers for toolsAllow diagnostics: ${String(error)}`,
);
return undefined;
}
}
/** Exported for #91613 keyless-inherited delivery-context regression coverage. */
export async function resolveCronDeliveryContext(params: {
cfg: OpenClawConfig;
@@ -476,6 +545,7 @@ type PreparedCronRunContext = {
modelFallbacksOverride?: string[];
thinkLevel: ThinkLevel | undefined;
timeoutMs: number;
preflightDiagnostics?: CronRunDiagnostics;
/**
* Set when the cron payload's `timeoutSeconds` was explicitly configured
* for this run (independent of whether its numeric value happens to equal
@@ -754,6 +824,22 @@ async function prepareCronRunContext(params: {
// `timeoutSeconds` happens to numerically equal `agents.defaults.timeoutSeconds`.
const runTimeoutOverrideMs = resolveCronRunTimeoutOverrideMs(explicitTimeoutSeconds);
const agentPayload = input.job.payload.kind === "agentTurn" ? input.job.payload : null;
const configuredProvider = cfgWithAgentDefaults.models?.providers?.[provider];
const modelApi =
findModelInCatalog(thinkingCatalog, provider, model)?.api ??
configuredProvider?.models.find((candidate) => candidate.id === model)?.api ??
configuredProvider?.api;
const preflightDiagnostics = await createCronToolsAllowPreflightDiagnostics({
cfg: cfgWithAgentDefaults,
jobId: input.job.id,
provider,
model,
modelApi,
agentId,
agentDir,
sessionKey: agentSessionKey,
agentPayload,
});
const { deliveryPlan, deliveryRequested, resolvedDelivery, sourceDelivery } =
await resolveCronDeliveryContext({
cfg: cfgWithAgentDefaults,
@@ -907,6 +993,7 @@ async function prepareCronRunContext(params: {
modelFallbacksOverride,
thinkLevel,
timeoutMs,
preflightDiagnostics,
runTimeoutOverrideMs,
},
};
@@ -1089,6 +1176,7 @@ async function finalizeCronRun(params: {
status: "error",
error: params.abortReason(),
diagnostics: mergeCronRunDiagnostics(
prepared.preflightDiagnostics,
createCronRunDiagnosticsFromAgentResult(finalRunResult, { finalStatus: "error" }),
createCronRunDiagnosticsFromError("cron-setup", params.abortReason()),
),
@@ -1121,6 +1209,7 @@ async function finalizeCronRun(params: {
status: "error",
error,
diagnostics: mergeCronRunDiagnostics(
prepared.preflightDiagnostics,
createCronRunDiagnosticsFromAgentResult(finalRunResult, { finalStatus: "error" }),
createCronRunDiagnosticsFromError("agent-run", error),
),
@@ -1138,6 +1227,7 @@ async function finalizeCronRun(params: {
const agentDiagnostics = createCronRunDiagnosticsFromAgentResult(finalRunResult, {
finalStatus: hasFatalErrorPayload ? "error" : "ok",
});
const runDiagnostics = mergeCronRunDiagnostics(prepared.preflightDiagnostics, agentDiagnostics);
const resolveRunOutcome = (result?: {
delivered?: boolean;
deliveryAttempted?: boolean;
@@ -1155,13 +1245,13 @@ async function finalizeCronRun(params: {
delivery: result?.delivery,
diagnostics: hasFatalErrorPayload
? mergeCronRunDiagnostics(
agentDiagnostics,
runDiagnostics,
createCronRunDiagnosticsFromError(
"agent-run",
embeddedRunError ?? "cron isolated run returned an error payload",
),
)
: agentDiagnostics,
: runDiagnostics,
...telemetry,
});
const failPendingPresentationWarningUnlessDelivered = (delivered?: boolean) => {
@@ -1265,7 +1355,7 @@ async function finalizeCronRun(params: {
deliveryResult.result.deliveryAttempted ?? deliveryResult.deliveryAttempted,
delivery: deliveryTrace,
diagnostics: mergeCronRunDiagnostics(
agentDiagnostics,
runDiagnostics,
deliveryResult.result.diagnostics,
deliveryResult.result.status === "error" && deliveryResult.result.error
? createCronRunDiagnosticsFromError("delivery", deliveryResult.result.error)
@@ -1484,9 +1574,12 @@ export async function runCronIsolatedAgentTurn(params: {
return prepared.context.withRunSession({
status: "error",
error,
diagnostics: createCronRunDiagnosticsFromError(
isCronLaneTimeout ? "cron-setup" : "agent-run",
isCronLaneTimeout ? error : err,
diagnostics: mergeCronRunDiagnostics(
prepared.context.preflightDiagnostics,
createCronRunDiagnosticsFromError(
isCronLaneTimeout ? "cron-setup" : "agent-run",
isCronLaneTimeout ? error : err,
),
),
});
} finally {
src/cron/run-diagnostics.test.ts
+38
View File
@@ -2,8 +2,10 @@
import { describe, expect, it } from "vitest";
import { setReplyPayloadMetadata } from "../auto-reply/reply-payload.js";
import {
createCronRunDiagnosticsFromMissingWebSearchProvider,
createCronRunDiagnosticsFromAgentResult,
createCronRunDiagnosticsFromError,
MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE,
mergeCronRunDiagnostics,
normalizeCronRunDiagnostics,
summarizeCronRunDiagnostics,
@@ -80,6 +82,42 @@ describe("cron run diagnostics", () => {
expect(summarizeCronRunDiagnostics(merged)).toBe("delivery failed");
});
it("warns when cron toolsAllow requests web_search without a provider", () => {
const diagnostics = createCronRunDiagnosticsFromMissingWebSearchProvider({
toolsAllow: ["web_*"],
hasWebSearchProvider: false,
nowMs: () => 900,
});
expect(diagnostics).toEqual({
summary: MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE,
entries: [
{
ts: 900,
source: "cron-preflight",
severity: "warn",
message: MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE,
toolName: "web_search",
},
],
});
});
it("does not warn for wildcard toolsAllow or configured web_search providers", () => {
expect(
createCronRunDiagnosticsFromMissingWebSearchProvider({
toolsAllow: ["*"],
hasWebSearchProvider: false,
}),
).toBeUndefined();
expect(
createCronRunDiagnosticsFromMissingWebSearchProvider({
toolsAllow: ["web_search"],
hasWebSearchProvider: true,
}),
).toBeUndefined();
});
it("keeps a later delivery error summary ahead of an earlier warning", () => {
const warning = normalizeCronRunDiagnostics({
summary: "agent warning",
src/cron/run-diagnostics.ts
+45
View File
@@ -1,5 +1,7 @@
/** Builds bounded, redacted diagnostics for cron run logs and UI surfaces. */
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
import { isToolAllowedByPolicyName } from "../agents/tool-policy-match.js";
import { normalizeToolName as normalizePolicyToolName } from "../agents/tool-policy.js";
import { getReplyPayloadMetadata } from "../auto-reply/reply-payload.js";
import { redactSensitiveText } from "../logging/redact.js";
import type {
@@ -13,6 +15,20 @@ const MAX_ENTRIES = 10;
const MAX_ENTRY_CHARS = 1_000;
const MAX_SUMMARY_CHARS = 2_000;
const EXEC_DIAGNOSTIC_TAIL_CHARS = 2_000;
const WEB_SEARCH_TOOL_NAME = "web_search";
export const MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE =
"web_search tool requested in toolsAllow but no web search provider is selected. Configure one with: openclaw configure --section web, or set tools.web.search.provider.";
export function toolsAllowRequestsWebSearch(toolsAllow?: string[]): boolean {
const explicitAllow = (toolsAllow ?? []).filter(
(entry) => normalizePolicyToolName(entry) !== "*",
);
return (
explicitAllow.length > 0 &&
isToolAllowedByPolicyName(WEB_SEARCH_TOOL_NAME, { allow: explicitAllow })
);
}
function normalizeSeverity(value: unknown): CronRunDiagnosticSeverity {
return value === "info" || value === "warn" || value === "error" ? value : "error";
@@ -230,6 +246,35 @@ export function createCronRunDiagnosticsFromError(
);
}
/** Reports a cron preflight warning for an explicitly allowed web_search with no provider. */
export function createCronRunDiagnosticsFromMissingWebSearchProvider(params: {
toolsAllow?: string[];
hasWebSearchProvider: boolean;
nowMs?: () => number;
}): CronRunDiagnostics | undefined {
if (params.hasWebSearchProvider || !params.toolsAllow || params.toolsAllow.length === 0) {
return undefined;
}
if (!toolsAllowRequestsWebSearch(params.toolsAllow)) {
return undefined;
}
return normalizeCronRunDiagnostics(
{
summary: MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE,
entries: [
{
ts: params.nowMs?.() ?? Date.now(),
source: "cron-preflight",
severity: "warn",
message: MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE,
toolName: WEB_SEARCH_TOOL_NAME,
},
],
},
{ nowMs: params.nowMs },
);
}
/** Extracts failed exec details from tool metadata into cron diagnostics. */
export function createCronRunDiagnosticsFromExecDetails(
details: unknown,