refactor: keep legacy web search config in doctor

This commit is contained in:
Peter Steinberger
2026-04-22 05:51:37 +01:00
parent a6dce7cf19
commit e20a5eeddb
8 changed files with 14 additions and 138 deletions

View File

@@ -138,42 +138,6 @@ describe("onboard-search provider resolution", () => {
).toBe("next-key");
});
it("uses provider-owned legacy search config readers generically", async () => {
const legacyEntry: PluginWebSearchProviderEntry = {
...createCustomProviderEntry(),
id: "legacy-search" as never,
pluginId: "legacy-search-plugin",
credentialPath: "plugins.entries.legacy-search-plugin.config.webSearch.apiKey",
getCredentialValue: (searchConfig) =>
(searchConfig?.legacySearch &&
typeof searchConfig.legacySearch === "object" &&
!Array.isArray(searchConfig.legacySearch)
? (searchConfig.legacySearch as Record<string, unknown>)
: undefined
)?.apiKey,
getConfiguredCredentialValue: () => undefined,
};
mocks.resolvePluginWebSearchProviders.mockImplementation((params) =>
params?.config ? [legacyEntry] : [],
);
const cfg: OpenClawConfig = {
tools: {
web: {
search: {
provider: "legacy-search" as never,
legacySearch: {
apiKey: "legacy-key",
},
} as never,
},
},
};
expect(mod.hasExistingKey(cfg, "legacy-search" as never)).toBe(true);
expect(mod.resolveExistingKey(cfg, "legacy-search" as never)).toBe("legacy-key");
});
it("uses config-aware non-bundled providers when building secret refs", async () => {
const customEntry = createCustomProviderEntry();
mocks.resolvePluginWebSearchProviders.mockImplementation((params) =>

View File

@@ -128,12 +128,8 @@ function providerIsReady(
}
function rawKeyValue(config: OpenClawConfig, provider: SearchProvider): unknown {
const search = config.tools?.web?.search;
const entry = resolveSearchProviderEntry(config, provider);
const configuredValue = entry?.getConfiguredCredentialValue?.(config);
return (
configuredValue ?? entry?.getCredentialValue(search as Record<string, unknown> | undefined)
);
return entry?.getConfiguredCredentialValue?.(config);
}
export function resolveExistingKey(

View File

@@ -72,7 +72,6 @@ export type RuntimeWebProviderSelectionParams<
setResolvedCredential: (params: {
resolvedConfig: OpenClawConfig;
provider: TProvider;
path: string;
value: string;
}) => void;
inactivePathsForProvider: (provider: TProvider) => string[];
@@ -373,7 +372,6 @@ export async function resolveRuntimeWebProviderSelection<
params.setResolvedCredential({
resolvedConfig: params.resolvedConfig,
provider,
path,
value: resolution.value,
});
}
@@ -386,7 +384,6 @@ export async function resolveRuntimeWebProviderSelection<
params.setResolvedCredential({
resolvedConfig: params.resolvedConfig,
provider,
path,
value: resolution.value,
});
break;

View File

@@ -901,7 +901,7 @@ describe("runtime web tools resolution", () => {
expect(resolvePluginWebSearchProvidersMock).not.toHaveBeenCalled();
});
it("limits legacy top-level web search apiKey auto-detect to compatibility owners", async () => {
it("does not auto-detect from legacy top-level web search apiKey", async () => {
const { metadata } = await runRuntimeWebTools({
config: asConfig({
tools: {
@@ -917,18 +917,9 @@ describe("runtime web tools resolution", () => {
},
});
expect(metadata.search.selectedProvider).toBe("brave");
expect(resolveManifestContractPluginIdsByCompatibilityRuntimePathMock).toHaveBeenCalledWith(
expect.objectContaining({
contract: "webSearchProviders",
path: "tools.web.search.apiKey",
origin: "bundled",
}),
);
expect(resolveBundledExplicitWebSearchProvidersFromPublicArtifactsMock).toHaveBeenCalledWith({
onlyPluginIds: ["brave"],
});
expect(resolveBundledWebSearchProvidersFromPublicArtifactsMock).not.toHaveBeenCalled();
expect(metadata.search.selectedProvider).toBe("duckduckgo");
expect(resolveManifestContractPluginIdsByCompatibilityRuntimePathMock).not.toHaveBeenCalled();
expect(resolveBundledExplicitWebSearchProvidersFromPublicArtifactsMock).not.toHaveBeenCalled();
expect(resolvePluginWebSearchProvidersMock).not.toHaveBeenCalled();
});
@@ -1030,10 +1021,6 @@ describe("runtime web tools resolution", () => {
expect(context.warnings).toEqual(
expect.arrayContaining([
expect.objectContaining({
code: "SECRETS_REF_IGNORED_INACTIVE_SURFACE",
path: "plugins.entries.brave.config.webSearch.apiKey",
}),
expect.objectContaining({
code: "SECRETS_REF_IGNORED_INACTIVE_SURFACE",
path: "plugins.entries.google.config.webSearch.apiKey",

View File

@@ -301,11 +301,9 @@ async function resolveSecretInputWithEnvFallback(params: {
function setResolvedWebSearchApiKey(params: {
resolvedConfig: OpenClawConfig;
provider: PluginWebSearchProviderEntry;
path: string;
value: string;
}): void {
const useLegacySearchConfig = params.path === "tools.web.search.apiKey";
if (params.provider.setConfiguredCredentialValue && !useLegacySearchConfig) {
if (params.provider.setConfiguredCredentialValue) {
params.provider.setConfiguredCredentialValue(params.resolvedConfig, params.value);
return;
}
@@ -416,8 +414,7 @@ function readConfiguredProviderCredential(params: {
config: OpenClawConfig;
search: Record<string, unknown> | undefined;
}): unknown {
const configuredValue = params.provider.getConfiguredCredentialValue?.(params.config);
return configuredValue ?? params.provider.getCredentialValue(params.search);
return params.provider.getConfiguredCredentialValue?.(params.config);
}
function inactivePathsForProvider(provider: PluginWebSearchProviderEntry): string[] {
@@ -432,7 +429,6 @@ function inactivePathsForProvider(provider: PluginWebSearchProviderEntry): strin
function setResolvedWebFetchApiKey(params: {
resolvedConfig: OpenClawConfig;
provider: PluginWebFetchProviderEntry;
path: string;
value: string;
}): void {
const tools = ensureObject(params.resolvedConfig as Record<string, unknown>, "tools");
@@ -561,25 +557,6 @@ export async function resolveRuntimeWebTools(params: {
diagnostics: [],
};
if (search || hasPluginWebSearchConfig) {
let searchCompatibilityOnlyPluginIds: string[] = [];
if (
!rawProvider &&
!hasPluginWebSearchConfig &&
isRecord(search) &&
Object.prototype.hasOwnProperty.call(search, "apiKey")
) {
const { resolveManifestContractPluginIdsByCompatibilityRuntimePath } =
await loadRuntimeWebToolsManifest();
searchCompatibilityOnlyPluginIds = resolveManifestContractPluginIdsByCompatibilityRuntimePath(
{
contract: "webSearchProviders",
path: "tools.web.search.apiKey",
origin: "bundled",
config: params.sourceConfig,
env,
},
);
}
const searchSurface = await resolveRuntimeWebProviderSurface({
contract: "webSearchProviders",
rawProvider,
@@ -596,12 +573,6 @@ export async function resolveRuntimeWebTools(params: {
sourceConfig: params.sourceConfig,
context: params.context,
configuredBundledPluginId,
onlyPluginIds:
configuredBundledPluginId === undefined &&
searchCompatibilityOnlyPluginIds.length > 0 &&
!(await getHasCustomWebSearchRisk())
? searchCompatibilityOnlyPluginIds
: undefined,
hasCustomWebSearchPluginRisk: await getHasCustomWebSearchRisk(),
}),
sortProviders: sortWebSearchProvidersForAutoDetect,
@@ -647,16 +618,10 @@ export async function resolveRuntimeWebTools(params: {
path,
envVars,
}),
setResolvedCredential: ({ resolvedConfig, provider, path, value }) =>
setResolvedCredential: ({ resolvedConfig, provider, value }) =>
setResolvedWebSearchApiKey({
resolvedConfig,
provider,
path:
search &&
Object.prototype.hasOwnProperty.call(search, "apiKey") &&
provider.getConfiguredCredentialValue?.(params.sourceConfig) == null
? "tools.web.search.apiKey"
: path,
value,
}),
inactivePathsForProvider,
@@ -747,11 +712,10 @@ export async function resolveRuntimeWebTools(params: {
envVars,
restrictEnvRefsToEnvVars: true,
}),
setResolvedCredential: ({ resolvedConfig, provider, path, value }) =>
setResolvedCredential: ({ resolvedConfig, provider, value }) =>
setResolvedWebFetchApiKey({
resolvedConfig,
provider,
path,
value,
}),
inactivePathsForProvider: inactivePathsForFetchProvider,

View File

@@ -54,6 +54,8 @@ const COVERAGE_LOADABLE_PLUGIN_ORIGINS =
buildCoverageLoadablePluginOrigins(COVERAGE_REGISTRY_ENTRIES);
const PLUGIN_OWNED_OPENCLAW_COVERAGE_EXCLUSIONS = new Set([
"channels.googlechat.accounts.*.serviceAccount",
// Doctor migrates legacy web search config into plugin-owned webSearch config.
"tools.web.search.apiKey",
"tools.web.fetch.firecrawl.apiKey",
]);

View File

@@ -122,7 +122,7 @@ describe("web search runtime", () => {
resolveRuntimeWebSearchProvidersMock.mockReturnValue([
createCustomSearchProvider({
credentialPath: "tools.web.search.custom.apiKey",
getCredentialValue: () => "configured",
requiresCredential: false,
}),
]);
@@ -155,39 +155,6 @@ describe("web search runtime", () => {
});
});
it("auto-detects a provider through its legacy search config reader", async () => {
const provider = createCustomSearchProvider({
getConfiguredCredentialValue: () => undefined,
getCredentialValue: (searchConfig) =>
(searchConfig?.customLegacy &&
typeof searchConfig.customLegacy === "object" &&
!Array.isArray(searchConfig.customLegacy)
? (searchConfig.customLegacy as Record<string, unknown>)
: undefined
)?.apiKey,
});
resolveRuntimeWebSearchProvidersMock.mockReturnValue([provider]);
resolvePluginWebSearchProvidersMock.mockReturnValue([provider]);
await expect(
runWebSearch({
config: {
tools: {
web: {
search: {
customLegacy: { apiKey: "legacy-key" },
} as never,
},
},
},
args: { query: "hello" },
}),
).resolves.toEqual({
provider: "custom",
result: { query: "hello", ok: true },
});
});
it("treats non-env SecretRefs as configured credentials for provider auto-detect", async () => {
const provider = createCustomSearchProvider();
resolveRuntimeWebSearchProvidersMock.mockReturnValue([provider]);

View File

@@ -71,9 +71,8 @@ function hasEntryCredential(
provider,
config,
toolConfig: search as Record<string, unknown> | undefined,
resolveRawValue: ({ provider: currentProvider, config: currentConfig, toolConfig }) =>
currentProvider.getConfiguredCredentialValue?.(currentConfig) ??
currentProvider.getCredentialValue(toolConfig),
resolveRawValue: ({ provider: currentProvider, config: currentConfig }) =>
currentProvider.getConfiguredCredentialValue?.(currentConfig),
resolveEnvValue: ({ provider: currentProvider, configuredEnvVarId }) =>
(configuredEnvVarId ? readWebProviderEnvValue([configuredEnvVarId]) : undefined) ??
readWebProviderEnvValue(currentProvider.envVars),