Web: derive search provider metadata from plugin contracts#50935
Web: derive search provider metadata from plugin contracts#50935gumadeiras merged 8 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR eliminates the duplicate
Confidence Score: 4/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/plugins/contracts/registry.ts
Line: 82-93
Comment:
**Exported mutable array for source-of-truth registry**
`bundledWebSearchPluginRegistrations` is now exported as a plain mutable `Array<...>`. Since it becomes the single source of truth for bundled web-search providers (consumed by both `bundled-web-search.ts` and the internal `bundledWebSearchPlugins` derivation), any caller that accidentally calls `.push()`, `.splice()`, or mutates an element would silently corrupt the shared state.
Consider typing it as `ReadonlyArray` to make the immutability contract explicit:
```suggestion
export const bundledWebSearchPluginRegistrations: ReadonlyArray<{
plugin: RegistrablePlugin;
credentialValue: unknown;
}> = [
{ plugin: bravePlugin, credentialValue: "BSA-test" },
{ plugin: firecrawlPlugin, credentialValue: "fc-test" },
{ plugin: googlePlugin, credentialValue: "AIza-test" },
{ plugin: moonshotPlugin, credentialValue: "sk-test" },
{ plugin: perplexityPlugin, credentialValue: "pplx-test" },
{ plugin: tavilyPlugin, credentialValue: "tvly-test" },
{ plugin: xaiPlugin, credentialValue: "xai-test" },
];
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "Web: derive search p..." |
There was a problem hiding this comment.
Pull request overview
This PR refactors web-search onboarding/configure/runtime paths to derive provider metadata directly from plugin web-search provider contracts and a shared bundled registry, reducing drift and fixing issues when selecting disabled bundled providers.
Changes:
- Switch onboarding/configure flows to consume
PluginWebSearchProviderEntrydirectly (instead of duplicating provider metadata). - Introduce a shared
bundledWebSearchPluginRegistrationsexport and rebuild bundled provider fast-path metadata from captured plugin registrations (including Tavily). - Align the
web_searchtool’s provider resolution to the shared runtime resolver.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/wizard/setup.finalize.ts | Updates finalize-step messaging to use provider id/envVars fields from the shared provider contract shape. |
| src/plugins/contracts/registry.ts | Exports bundledWebSearchPluginRegistrations as a shared source of truth for bundled web-search plugin registrations. |
| src/plugins/bundled-web-search.ts | Replaces duplicated bundled provider descriptors with provider entries derived from captured plugin registrations. |
| src/commands/onboard-search.ts | Refactors search onboarding to use PluginWebSearchProviderEntry and new selection helpers. |
| src/commands/onboard-search.test.ts | Adds coverage for disabled bundled-provider re-enable + persistence behavior and updates expectations for id-based options. |
| src/commands/configure.wizard.ts | Updates configure wizard web-search flow to use the new id contract + apply provider-selection side effects. |
| src/commands/configure.wizard.test.ts | Adds mocks + tests asserting provider-owned config returned from apply functions is persisted. |
| src/agents/tools/web-search.ts | Adjusts internal testing helper typing to match the shared runtime resolver signature. |
Comments suppressed due to low confidence (1)
src/commands/onboard-search.ts:42
resolveSearchProviderEntry()only searchesSEARCH_PROVIDER_OPTIONS, which is built without aconfig/workspaceDirand therefore only reflects the default bundled provider set. As a result,rawKeyValue()/hasExistingKey()/applySearchKey()can fail for non-bundled or config-dependent web search providers (e.g., providers coming fromplugins.installs/plugins.load.paths), causing existing keys to be missed and new keys not to be written via the provider’s setters.
Suggestion: when an entry isn’t found in SEARCH_PROVIDER_OPTIONS, fall back to resolving providers with resolvePluginWebSearchProviders({ config, workspaceDir, env, bundledAllowlistCompat: true, ... }) using the current config (or otherwise accept config in resolveSearchProviderEntry). This preserves support for custom providers while keeping the bundled fast-path list for the common case.
function resolveSearchProviderEntry(
provider: SearchProvider,
): PluginWebSearchProviderEntry | undefined {
return SEARCH_PROVIDER_OPTIONS.find((entry) => entry.id === provider);
}
export function hasKeyInEnv(entry: Pick<PluginWebSearchProviderEntry, "envVars">): boolean {
return entry.envVars.some((k) => Boolean(process.env[k]?.trim()));
}
function rawKeyValue(config: OpenClawConfig, provider: SearchProvider): unknown {
const search = config.tools?.web?.search;
const entry = resolveSearchProviderEntry(provider);
return (
entry?.getConfiguredCredentialValue?.(config) ??
entry?.getCredentialValue(search as Record<string, unknown> | undefined)
);
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8180cf32e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
b8180cf to
c3c3569
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d575f1085
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb06375a1b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 603d16faed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -483,23 +484,36 @@ export async function finalizeSetupWizard( | |||
|
|
|||
| const webSearchProvider = nextConfig.tools?.web?.search?.provider; | |||
| const webSearchEnabled = nextConfig.tools?.web?.search?.enabled; | |||
| const runtimeSearchProviders = listWebSearchProviders({ config: nextConfig }); | |||
There was a problem hiding this comment.
Resolve final wizard provider notes from
nextConfig
When the setup wizard is run through the Gateway/Control UI, this process already has an active plugin registry from loadOpenClawPlugins in src/gateway/server-plugins.ts:408-422. listWebSearchProviders then delegates to resolveRuntimeWebSearchProviders, which ignores the passed config whenever getActivePluginRegistry().webSearchProviders is non-empty (src/plugins/web-search-providers.runtime.ts:46-52). As a result, a provider that this wizard just enabled in nextConfig can still be missing here, so the summary reports it as unavailable and skips key detection until after a reload/restart. This summary path needs a config-snapshot lookup, not the already-active runtime registry.
Useful? React with 👍 / 👎.
cfb3bdd to
e1ef201
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1ef201bd5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| nextSearch = { | ||
| ...existingSearch, | ||
| enabled: false, | ||
| }; |
There was a problem hiding this comment.
Preserve existing web_search state when no providers are available
When resolveSearchProviderOptions(nextConfig) is empty (for example plugins.enabled=false or every provider is denied), this branch rewrites tools.web.search.enabled to false and the wizard later persists that change. In that configuration, running openclaw configure --section web just to inspect settings or change web_fetch silently turns off an otherwise valid search setup, so once the plugin policy is relaxed the stored provider/key still exists but web_search remains disabled until the user notices and re-enables it manually.
Useful? React with 👍 / 👎.
60be8a7 to
e1c7d72
Compare
…50935) Merged via squash. Prepared head SHA: e1c7d72 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
…50935) Merged via squash. Prepared head SHA: e1c7d72 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
…50935) Merged via squash. Prepared head SHA: e1c7d72 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
…50935) Merged via squash. Prepared head SHA: e1c7d72 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Summary
Describe the problem and fix in 2–5 bullets:
PluginWebSearchProviderEntrycontract, the shared bundled web-search registry is the single source of truth for bundled fast-path metadata, the agentweb_searchtool uses the shared runtime resolver, and the shared bundled registry now includes Tavily frommain.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
Security Impact (required)
Yes/No) NoYes/No) YesYes/No) NoYes/No) NoYes/No) NoYes, explain risk + mitigation:This changes where existing web-search credentials are read/written during onboarding/configure, but it narrows behavior onto existing provider contracts instead of adding a new credential surface. Validation covers provider-owned config persistence and disabled-provider re-enable flows.
Repro + Verification
Environment
plugins.entries.<id>.config.webSearch.apiKeySteps
tools.web.search, the plugin enable bit, and the provider-owned plugin config after the flow.Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
pnpm tsgo;OPENCLAW_TEST_PROFILE=low OPENCLAW_TEST_SERIAL_GATEWAY=1 pnpm test -- src/plugins/bundled-web-search.test.ts src/plugins/web-search-providers.test.ts src/config/config.web-search-provider.test.ts;OPENCLAW_TEST_PROFILE=low OPENCLAW_TEST_SERIAL_GATEWAY=1 pnpm test -- src/agents/tools/web-tools.enabled-defaults.test.ts;OPENCLAW_TEST_PROFILE=low OPENCLAW_TEST_SERIAL_GATEWAY=1 pnpm test -- src/commands/configure.wizard.test.ts; directbun run -exercise of disabled-Firecrawl interactive + quickstart onboarding paths and currentSEARCH_PROVIDER_OPTIONSmembership including Tavily.main.src/commands/onboard-search.test.tsstill hangs locally in the repo’s existing Vitest harness pattern and did not produce a clean completion signal on this machine.Review Conversations
Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoFailure Recovery (if this breaks)
src/commands/onboard-search.ts,src/commands/configure.wizard.ts,src/plugins/bundled-web-search.ts,src/plugins/contracts/registry.tsweb_searchselecting a different provider than runtime resolution.Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.bundledWebSearchPluginRegistrations.src/commands/onboard-search.test.ts.