Skip to content

fix(plugins): default web-provider runtime cache to true#75992

Closed
DmitryPogodaev wants to merge 2 commits intoopenclaw:mainfrom
DmitryPogodaev:fix/web-provider-default-cache-true
Closed

fix(plugins): default web-provider runtime cache to true#75992
DmitryPogodaev wants to merge 2 commits intoopenclaw:mainfrom
DmitryPogodaev:fix/web-provider-default-cache-true

Conversation

@DmitryPogodaev
Copy link
Copy Markdown
Contributor

Summary

resolveWebProviderLoadOptions and the setup-mode loadOpenClawPlugins call inside resolvePluginWebProviders default to cache: false. Every dispatch that resolves bundled web search/fetch providers (brave, google, ollama, etc.) re-imports the same plugin modules even when the requested set has not changed.

Repro

  1. Configure any agent that touches web search providers via the standard runtime path (default for most setups; bundled provider plugins available include brave, duckduckgo, exa, firecrawl, google, minimax, moonshot, ollama, perplexity, searxng, tavily, xai).
  2. Send any message.
  3. Observe in [plugins] debug logs:
[plugins] loading brave from .../extensions/brave/index.js
[plugins] loading duckduckgo from .../extensions/duckduckgo/index.js
... 10 more ...
[plugins] loaded 12 plugin(s) (12 attempted) in ~1500ms

This repeats verbatim on every dispatch, even on a fully warm gateway with stable config.

Fix

Flip the default from false to true. The cache key already varies by onlyPluginIds, activate, and the other relevant inputs (see buildCacheKey), so subsequent calls with the same effective inputs correctly hit the cached registry.

Affected lines:

// resolveWebProviderLoadOptions
cache: params.cache ?? false,  // → true

// resolvePluginWebProviders setup-mode
cache: params.cache ?? false,  // → true

Callers that explicitly pass cache: false (auth-choice catalog warning, etc.) keep their existing override.

Test plan

  • Existing web-provider runtime tests.
  • On a deployment with web search providers configured, send several messages and verify the 12-plugin web search load batch fires only once (gateway lifetime).

🤖 Patch authored after debugging slow message dispatch on a personal VPS.

DmitryPogodaev added 2 commits May 2, 2026 14:21
Restores the fix originally landed in openclaw#61854 (issue openclaw#61756).
runtimeSubagentMode does not influence which plugins are loaded or how
they are configured — it is metadata stored alongside the active registry
state. Including it in the cache key causes redundant register() calls
when call sites in the message processing pipeline pass inconsistent
allowGatewaySubagentBinding values, resulting in repeated full plugin
reloads on every message dispatch.

Observed symptom: bundled provider plugin (e.g. "openai") imported 9-10
times per single message dispatch (~120ms each = ~1.2s wasted), even on
a fully warm gateway with stable config.
resolveWebProviderLoadOptions and the setup-mode loadOpenClawPlugins
call in resolvePluginWebProviders default to cache: false. Every dispatch
that resolves bundled web search/fetch providers (e.g. brave, google,
ollama, etc.) repeats the full module-import sequence even when the
plugin set is unchanged.

Symptom on a stable config: the 12-plugin web search batch
(brave, duckduckgo, exa, firecrawl, google, minimax, moonshot, ollama,
perplexity, searxng, tavily, xai) is re-imported on every message
dispatch, ~1.5s per dispatch.

Cache key already correctly varies by onlyPluginIds, activate, and other
relevant inputs, so flipping the default to true is safe and produces
correct cache hits for repeated calls.
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 2, 2026

Codex review: found issues before merge.

Summary
The PR makes web search/fetch provider resolution use cached plugin loads by default and removes runtimeSubagentMode from the plugin-loader cache key.

Reproducibility: yes. The code-level reproduction is to apply the cache-key removal and run the existing runtime-registry compatibility case that expects explicit subagent requests not to reuse a gateway-bindable registry.

Next step before merge
Maintainer review is needed because the remaining blocker crosses the plugin subagent runtime isolation boundary; the safe path is a narrower branch that keeps the cache-key discriminator.

Security
Needs attention: The diff changes a security-sensitive plugin runtime cache boundary by allowing subagent binding modes to collide in the loader key.

Review findings

  • [P2] Keep subagent mode in the loader cache key — src/plugins/loader.ts:686
  • [P3] Add the required changelog entry — src/plugins/web-provider-runtime-shared.ts:115
Review details

Best possible solution:

Keep runtimeSubagentMode in the loader cache key, limit the implementation to the web-provider cache defaults with focused regression coverage, and add the required changelog entry.

Do we have a high-confidence way to reproduce the issue?

Yes. The code-level reproduction is to apply the cache-key removal and run the existing runtime-registry compatibility case that expects explicit subagent requests not to reuse a gateway-bindable registry.

Is this the best way to solve the issue?

No. Defaulting web-provider cache use may be the right narrow fix, but removing the subagent-mode discriminator is not the safest way because current main already has a narrower default-to-gateway-bindable reuse path.

Full review comments:

  • [P2] Keep subagent mode in the loader cache key — src/plugins/loader.ts:686
    Removing runtimeSubagentMode from buildCacheKey makes gateway-bindable, default, and explicit-subagent loads eligible for the same compatibility/cache key. Existing coverage expects an explicit subagent request not to reuse a gateway-bindable registry because that changes the PluginRuntime.subagent surface, so please leave this discriminator in place and limit the PR to the web-provider cache defaults.
    Confidence: 0.9
  • [P3] Add the required changelog entry — src/plugins/web-provider-runtime-shared.ts:115
    This is a user-visible plugin/web-provider dispatch performance fix, but the PR file list does not include CHANGELOG.md. Repo policy requires a single-line Unreleased entry before merge.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.88

Security concerns:

  • [medium] Subagent runtime cache key collision — src/plugins/loader.ts:686
    Removing runtimeSubagentMode can let an explicit-subagent request reuse a gateway-bindable cached registry, bypassing the current separation between explicit, gateway-bound, and unavailable subagent runtimes.
    Confidence: 0.84

What I checked:

  • Current web-provider defaults: Current main still passes cache: params.cache ?? false for both runtime and setup-mode web-provider loads, so the cache-default part is not already implemented verbatim. (src/plugins/web-provider-runtime-shared.ts:115, 93e2d90af174)
  • Cache key currently separates subagent modes: Current buildCacheKey includes runtimeSubagentMode; the PR removes that discriminator from the returned key. (src/plugins/loader.ts:674, 93e2d90af174)
  • Existing compatibility test: The runtime-registry test expects a gateway-bindable active registry to be reusable for default runtime options but not for an explicit subagent runtime, which depends on the subagent mode remaining compatibility-shaping. (src/plugins/loader.runtime-registry.test.ts:107, 93e2d90af174)
  • Runtime boundary: createLateBindingSubagent resolves an explicit subagent first, gateway binding only when opted in, and otherwise an unavailable fallback, so mixing these cache modes changes the plugin runtime surface. (src/plugins/runtime/index.ts:155, 93e2d90af174)
  • Recent area ownership: Recent current-main history for the web-provider runtime includes perf(plugins): reuse active web provider registry and fix: stabilize release web provider validation, both touching the same runtime file family. (src/plugins/web-provider-runtime-shared.ts:124, 01bd2f2ecc2a)

Likely related people:

  • Peter Steinberger: Authored the recent current-main web-provider registry reuse and validation commits touching src/plugins/web-provider-runtime-shared.ts and its tests. (role: recent maintainer; confidence: high; commits: 01bd2f2ecc2a, 44778bc7e2a6; files: src/plugins/web-provider-runtime-shared.ts, src/plugins/web-provider-runtime-shared.test.ts)
  • Shakker: Current shallow blame points the loader cache-key and runtime-registry compatibility test area to this author; provenance is weaker because the available local history is grafted around this range. (role: adjacent owner; confidence: low; commits: d3f9bed1c3ac; files: src/plugins/loader.ts, src/plugins/loader.runtime-registry.test.ts)

Remaining risk / open question:

  • No live dispatch profiling was run in this read-only review, so the exact remaining performance gain from the web-provider cache-default change is unmeasured.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 93e2d90af174.

@steipete
Copy link
Copy Markdown
Contributor

steipete commented May 2, 2026

Thanks for tracking this down. I landed the safe part directly on main in d13a206.

What landed:

  • src/plugins/web-provider-runtime-shared.ts now defaults repeated runtime and setup web-provider plugin loads to cache: true.
  • Explicit cache: false callers still opt out.
  • Added focused coverage for runtime/setup defaults and explicit opt-outs in src/plugins/web-provider-runtime-shared.test.ts.
  • Added the changelog entry crediting this PR.

I did not take the src/plugins/loader.ts cache-key change because it would merge default, explicit-subagent, and gateway-bindable runtime modes into the same plugin registry cache key. That crosses the subagent runtime isolation boundary and matches the review concern above, so keeping the fix limited to the web-provider cache default is the safer shape.

Verification:

  • pnpm test src/plugins/web-provider-runtime-shared.test.ts
  • pnpm check:changelog-attributions
  • Testbox OPENCLAW_TESTBOX=1 pnpm check:changed on tbx_01kqksgxh3bma4dj5c4km952fm

@steipete
Copy link
Copy Markdown
Contributor

steipete commented May 2, 2026

Superseded by d13a206.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants