fix(onboard): pin Brave web-search plugin to OPENCLAW_VERSION#4955
Open
johnellison wants to merge 1 commit into
Open
fix(onboard): pin Brave web-search plugin to OPENCLAW_VERSION#4955johnellison wants to merge 1 commit into
johnellison wants to merge 1 commit into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR extends the build-time plugin installer to treat the Brave web-search provider as an external plugin that must be pinned to ChangesWeb-search plugin support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/openclaw-build-messaging-plugins.py (1)
136-140: ⚡ Quick winInclude Brave API key placeholder in doctor env overrides when web search is enabled.
doctor_env_overrides(...)is still channel-only. WithNEMOCLAW_WEB_SEARCH_ENABLED=1, the generated config usesopenshell:resolve:env:BRAVE_API_KEY; not passing that placeholder intoopenclaw doctor --fixrisks doctor mutating the web-search block unexpectedly.Suggested patch
-def doctor_env_overrides(channels: Iterable[str]) -> dict[str, str]: +def doctor_env_overrides( + channels: Iterable[str], + *, + web_search_enabled: bool, +) -> dict[str, str]: overrides: dict[str, str] = {} for channel in channels: overrides.update(DOCTOR_ENV_BY_CHANNEL.get(channel, {})) + if web_search_enabled: + overrides["BRAVE_API_KEY"] = "openshell:resolve:env:BRAVE_API_KEY" return overrides @@ - env_overrides = doctor_env_overrides(channels) + env_overrides = doctor_env_overrides( + channels, web_search_enabled=web_search_enabled + )Also applies to: 173-197
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/openclaw-build-messaging-plugins.py` around lines 136 - 140, doctor_env_overrides currently only aggregates channel-specific entries (DOCTOR_ENV_BY_CHANNEL) and omits the Brave API key placeholder when web search is enabled; update doctor_env_overrides to also inject the BRAVE_API_KEY placeholder into the returned dict when the environment flag NEMOCLAW_WEB_SEARCH_ENABLED (or equivalent config) is truthy so that openshell:resolve:env:BRAVE_API_KEY is present for openclaw doctor --fix; modify the same logic area referenced further down (the other doctor env aggregation block) to apply the same addition so both places include the Brave key placeholder when web search is enabled, using the existing symbols doctor_env_overrides, DOCTOR_ENV_BY_CHANNEL, and the BRAVE_API_KEY placeholder name.test/openclaw-build-messaging-plugins.test.ts (1)
141-153: ⚡ Quick winAdd a doctorEnv assertion for
BRAVE_API_KEYin the web-search-enabled test.This test already validates install pinning; adding a
doctorEnvassertion will lock the end-to-end contract and prevent regressions indoctor --fixinput shaping.Suggested assertion
it("pins the Brave web-search plugin to OPENCLAW_VERSION when web search is enabled", () => { @@ expect(payload.webSearchEnabled).toBe(true); + expect(payload.doctorEnv).toMatchObject({ + BRAVE_API_KEY: "openshell:resolve:env:BRAVE_API_KEY", + }); expect(payload.installSpecs).toEqual([ "npm:`@openclaw/slack`@2026.5.22", "npm:`@openclaw/brave-plugin`@2026.5.22", ]); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/openclaw-build-messaging-plugins.test.ts` around lines 141 - 153, The test "pins the Brave web-search plugin to OPENCLAW_VERSION when web search is enabled" currently checks install pinning but omits verifying doctorEnv; modify that test (the one constructing payload via parseDryRun and asserting payload.webSearchEnabled/installSpecs) to also assert that payload.doctorEnv includes a BRAVE_API_KEY entry when NEMOCLAW_WEB_SEARCH_ENABLED is set, e.g., add an assertion on payload.doctorEnv (using payload.doctorEnv or payload.doctorEnv.BRAVE_API_KEY) to ensure BRAVE_API_KEY is present and non-empty to lock the doctor --fix contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/openclaw-build-messaging-plugins.py`:
- Around line 136-140: doctor_env_overrides currently only aggregates
channel-specific entries (DOCTOR_ENV_BY_CHANNEL) and omits the Brave API key
placeholder when web search is enabled; update doctor_env_overrides to also
inject the BRAVE_API_KEY placeholder into the returned dict when the environment
flag NEMOCLAW_WEB_SEARCH_ENABLED (or equivalent config) is truthy so that
openshell:resolve:env:BRAVE_API_KEY is present for openclaw doctor --fix; modify
the same logic area referenced further down (the other doctor env aggregation
block) to apply the same addition so both places include the Brave key
placeholder when web search is enabled, using the existing symbols
doctor_env_overrides, DOCTOR_ENV_BY_CHANNEL, and the BRAVE_API_KEY placeholder
name.
In `@test/openclaw-build-messaging-plugins.test.ts`:
- Around line 141-153: The test "pins the Brave web-search plugin to
OPENCLAW_VERSION when web search is enabled" currently checks install pinning
but omits verifying doctorEnv; modify that test (the one constructing payload
via parseDryRun and asserting payload.webSearchEnabled/installSpecs) to also
assert that payload.doctorEnv includes a BRAVE_API_KEY entry when
NEMOCLAW_WEB_SEARCH_ENABLED is set, e.g., add an assertion on payload.doctorEnv
(using payload.doctorEnv or payload.doctorEnv.BRAVE_API_KEY) to ensure
BRAVE_API_KEY is present and non-empty to lock the doctor --fix contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8e553946-5bdc-4c80-9630-a6015daf3ec6
📒 Files selected for processing (2)
scripts/openclaw-build-messaging-plugins.pytest/openclaw-build-messaging-plugins.test.ts
The Brave web-search provider is an external plugin (@openclaw/brave-plugin),
like the messaging channels and diagnostics OTEL exporter — but it was never
pinned. `openclaw doctor --fix` installs it from the official catalog's
unversioned npmSpec, so it resolves to npm `latest`. Once OpenClaw publishes a
release the NemoClaw OPENCLAW_VERSION pin has not caught up to, `latest` drifts
ahead of the host: the newer plugin imports plugin-SDK symbols the older host
does not export, so web_search fails at runtime with
(0 , _providerWebSearch.readPositiveIntegerParam) is not a function
This is timing-dependent — it only breaks once `latest` has drifted past the
pin — which is why it reproduces intermittently.
Pin and build-install @openclaw/brave-plugin to OPENCLAW_VERSION when
NEMOCLAW_WEB_SEARCH_ENABLED is set, mirroring the existing messaging-channel and
diagnostics-OTEL handling. NEMOCLAW_WEB_SEARCH_ENABLED is already plumbed into
this build step (Dockerfile ARG + ENV), so no Dockerfile change is needed. The
plugin publishes a build for every core calver, so the pin always resolves.
Addresses NVIDIA#3948
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: John Ellison <john@john-ellison.com>
johnellison
force-pushed
the
fix/pin-brave-web-search-plugin
branch
from
25389e0 to
b57664b
Compare
wscurran
added
bug-fix
Contributor
|
✨ Thanks for submitting this detailed PR about pinning the Brave web-search plugin to OPENCLAW_VERSION. This proposes a way to fix the intermittent failure in the onboarding flow. Related open issues: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Brave web-search provider is an external plugin (
@openclaw/brave-plugin) — exactly like the messaging-channel plugins and the diagnostics-OTEL exporter thatscripts/openclaw-build-messaging-plugins.pyalready pins toOPENCLAW_VERSION. But the Brave plugin was never pinned.openclaw doctor --fixinstalls it from the official catalog's unversioned npmSpec, so it resolves to npmlatest.Once OpenClaw publishes a release the NemoClaw
OPENCLAW_VERSIONpin hasn't caught up to,latestdrifts ahead of the host. The newer plugin imports plugin-SDK symbols the older host doesn't export, soweb_searchfails at runtime with:Because it only breaks once
latesthas drifted past the pin, it reproduces intermittently — which matches the back-and-forth in #3948.Concrete timeline
2026.5.22published2026.6.1publishedopenclaw@openclaw/brave-plugin@openclaw/brave-plugin@latestis now2026.6.1(peerDependencies.openclaw: ">=2026.6.1").maincurrently pinsOPENCLAW_VERSION=2026.5.27→ host2026.5.27+ plugin2026.6.1= incompatible. Every web-search onboard since Jun 3 is affected.Fix
Pin and install
@openclaw/brave-plugin@$OPENCLAW_VERSIONwhenNEMOCLAW_WEB_SEARCH_ENABLEDis set, mirroring the existing messaging-channel and diagnostics-OTEL handling in the same script.NEMOCLAW_WEB_SEARCH_ENABLEDis already anARGexported into the buildENVblock this script runs under (Dockerfile), so no Dockerfile change is needed. The plugin publishes a build for every core calver (e.g.@openclaw/brave-plugin@2026.5.27exists), so the pin always resolves.Tests
Added cases mirroring the diagnostics-OTEL tests:
@openclaw/brave-plugin@$OPENCLAW_VERSIONwhen web search is enabledOPENCLAW_VERSIONwhen web search is enabledVerified the script's
--dry-runoutput and the--pininstall ordering against each new assertion locally; the existing messaging/OTEL cases are unaffected.Verification
On a live sandbox, swapping the drifted
2026.6.1plugin for theOPENCLAW_VERSION-matched build makesweb_searchreturn real Brave results with no SDK error. A directcurltoapi.search.brave.comwas already returning200throughout — i.e. the API key and network policy were never the problem; only the plugin/host version skew.Scope
Deliberately limited to the version-pin root cause. While investigating I also checked whether the build-time
doctor --fix(which runs withoutBRAVE_API_KEYin its env) strips thetools.web.search.apiKeyplaceholder. On current OpenClaw I could not reproduce that — doctor preserves/relocates the placeholder rather than dropping it — so I left it out rather than bundle an unverified change. Happy to revisit if maintainers have seen the apiKey actually go missing.Addresses #3948.
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Tests