fix(onboard): pin Brave web-search plugin and preserve its apiKey through build doctor#4955
Conversation
📝 WalkthroughWalkthroughTreats the Brave web-search provider as an optional external plugin that must be pinned to OPENCLAW_VERSION when enabled; threads a web_search_enabled flag through validation, spec generation, doctor env overrides, and adds tests plus a webSearchEnabled dry-run field. 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 |
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>
25389e0 to
b57664b
Compare
|
✨ 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: |
…ctor The generated config sets tools.web.search.apiKey to openshell:resolve:env:BRAVE_API_KEY, but the build-time `openclaw doctor --fix` runs with only the messaging-channel placeholders in its env. Without BRAVE_API_KEY present, doctor can mutate/strip the web-search block while repairing the config, leaving Brave search without a usable key at runtime. Inject the BRAVE_API_KEY placeholder into the doctor env when web search is enabled, the same way the channel bot tokens are already injected, so doctor preserves the resolvable web-search config. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: John Ellison <john@john-ellison.com>
|
Updated to address @coderabbitai's review. The PR now covers both root causes of #3948:
Tests now assert the |
|
Tip For best results, initiate chat on the files or code changes.
The test coverage for both the (๑˃ᴗ˂)ﻭ ✨ |
prekshivyas
left a comment
There was a problem hiding this comment.
LGTM on the code — well-reasoned and it addresses both halves of #3948:
- Pins
@openclaw/brave-plugintoOPENCLAW_VERSION(same pattern as messaging channels + diagnostics-OTEL), preventing the unversioned-catalog drift where a newer plugin imports SDK symbols the pinned host lacks (readPositiveIntegerParam is not a function) → web_search crashes → 'agent claims no web access'. - Injects
BRAVE_API_KEY=openshell:resolve:env:BRAVE_API_KEYinto thedoctor --fixenv so thetools.web.search.apiKeyblock isn't stripped — directly fixing the 'Brave key not usable' symptom. (The raw-curl 422 in the report is expected — curl doesn't carry the token.)
Test added, logic is sound.
One thing before merge: CI currently shows only 4 checks (the python/test suite appears to have aged out on this older PR). Recommend a rebase/main-merge to re-trigger the full suite and confirm green before merging.
Run Biome on the new test so static-checks' formatter hook leaves it unchanged (it was collapsing a multi-line expect()). Formatting only; no behavior change. Co-authored-by: johnellison <john@john-ellison.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@johnellison one small thing left to make CI green:
That's all it checks — once that line is in the description, (I already pushed a tiny Biome-format commit to clear |
|
@prekshivyas Done — added the |
## Summary - Add v0.0.64 release notes from the release announcement and link them to the relevant deeper docs. - Document that custom policy presets recorded through `policy-add --from-file` and `--from-dir` survive snapshot restore and sandbox recreation. - Refresh generated NemoClaw user skills from the current source docs. ## Source summary - #5104 -> `docs/manage-sandboxes/backup-restore.mdx`, `docs/network-policy/customize-network-policy.mdx`: Documents custom policy presets preserved through snapshot restore. - #4955 -> `docs/about/release-notes.mdx`: Adds release-note coverage for Brave web-search pinning and `BRAVE_API_KEY` placeholder preservation. - #5116, #5269 -> `docs/about/release-notes.mdx`: Adds release-note coverage for Docker-driver gateway health and rootfs guard stability. - #5241, #5085 -> `docs/about/release-notes.mdx`: Adds release-note coverage for chat-completions provider selection and Nemotron Ultra 550B tool-less request compatibility. - #5268, #5210, #5257 -> `docs/about/release-notes.mdx`: Adds release-note coverage for messaging render plan refresh, OpenClaw scope-upgrade approval recovery, and Hermes WhatsApp bridge dependency setup. - Current source docs -> `.agents/skills/`: Regenerates user-skill references so agent-facing guidance matches the source documentation. ## Verification - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` - `npm run docs` - `npm run build:cli` - `npm run typecheck:cli` - Commit/pre-push hooks: markdownlint, gitleaks, docs-to-skills verification, TypeScript CLI, and skills YAML checks passed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified sandbox snapshot restore preserves custom policy presets and restores them without original files. * Switched sandbox setup and remote deployment guidance to Docker-based workflows and emphasized remote onboarding flow. * Expanded troubleshooting for gateway recovery, Docker GPU/WSL issues, and onboarding resume. * Added/updated CLI docs: advanced maintenance, session export, upload/download wrappers, and status recovery guidance. * Added v0.0.64 release notes and links to NemoClaw Community; fixed command reference formatting. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…tall validates (#5318) ## Summary Enabling Brave Web Search at onboarding still aborts the sandbox image build at the OpenClaw plugin-install step. `scripts/generate-openclaw-config.mts` writes the legacy `tools.web.search` shape (inline `apiKey`), which OpenClaw 2026.5.x rejects — and because `openclaw plugins install` validates the existing config *before* installing, it exits non-zero while the brave plugin is not yet installed, aborting the build under `set -eu` before `openclaw doctor --fix` can migrate it. This PR emits the current schema directly so the build-time install validates cleanly. ## Related Issue Fixes #5266 (follow-up to #4955 / #3948) ## Changes - `scripts/generate-openclaw-config.mts`: when `NEMOCLAW_WEB_SEARCH_ENABLED=1`, write `tools.web.search = { enabled, provider: "brave" }` and move the provider-owned `apiKey` placeholder to `plugins.entries.brave.config.webSearch` — the same shape `doctor --fix` produces after a successful install. - `test/generate-openclaw-config.test.ts`: update the existing web-search case to assert the current schema (no inline `apiKey`) and the `plugins.entries.brave` entry. ## Why this is the fix `#4955` pinned/installed `@openclaw/brave-plugin` at build time and threaded `BRAVE_API_KEY` into the doctor env, assuming `doctor --fix` would migrate the legacy block. But `plugins install` runs config validation up front, so on the legacy shape it fails (exit 1) before the plugin exists — an ordering deadlock the migration step never gets to resolve. Verified inside `ghcr.io/nvidia/nemoclaw/sandbox-base:v0.0.55` (OpenClaw 2026.5.27): ``` legacy schema (inline apiKey) → openclaw plugins install EXIT=1 # current behavior, breaks build current schema (plugins.entries.brave) → openclaw plugins install EXIT=0 # this PR ``` End-to-end: a real `nemoclaw onboard --recreate-sandbox` with Brave Web Search **and** Slack both enabled now builds cleanly, and `web_search` works in the running sandbox alongside Slack. ## Type of Change - [x] Code change (feature, bug fix, or refactor) ## Verification - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] `test/generate-openclaw-config.test.ts` passes (128/128) — covers the changed code - [x] `npm run typecheck:cli` passes; `npx biome lint` clean on changed files - [ ] Full `npm test` — relevant subset verified; the full local run had unrelated timeouts under heavy parallelism (spawn/docker suites), none in the changed files. Deferring to CI for the authoritative full run. --- Signed-off-by: Zhi Yan Liu <lzy.dev@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Web-search config updated to the new OpenClaw schema: provider-only tools entry and Brave API key moved into plugin config; CI test-size budget adjusted. * **Tests** * Updated tests to assert the new schema and added coverage to accept legacy inline API-key placements. * **Bug Fixes** * Web-search verification now resolves and validates the Brave API key from both new plugin config and legacy inline locations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Zhi Yan Liu <lzy.dev@gmail.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Summary
Two related defects break Brave
web_searchout of the box when web search is enabled. Both stem from the same gap: the Brave web-search provider is an external plugin (@openclaw/brave-plugin) — exactly like the messaging channels and the diagnostics-OTEL exporter thatscripts/openclaw-build-messaging-plugins.pyalready pins and prepares — but it was never given the same treatment.1. Unpinned plugin drifts ahead of the host (the runtime crash)
Without an explicit pin,
openclaw doctor --fixinstalls the plugin from the official catalog's unversioned npmSpec, so it resolves to npmlatest. Once OpenClaw publishes a release the NemoClawOPENCLAW_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 "can't reproduce on my side" back-and-forth in #3948.Timeline:
2026.5.22published2026.6.1publishedopenclaw@openclaw/brave-plugin@openclaw/brave-plugin@latestis now2026.6.1(peerDependencies.openclaw: ">=2026.6.1").mainpinsOPENCLAW_VERSION=2026.5.27→ host2026.5.27+ plugin2026.6.1= incompatible. Every web-search onboard since Jun 3 is affected.2. Build-time doctor can strip the Brave apiKey placeholder
generate-openclaw-config.mtswritestools.web.search.apiKey = "openshell:resolve:env:BRAVE_API_KEY", but the build-timeopenclaw doctor --fixruns with only the messaging-channel placeholders in its env. WithBRAVE_API_KEYabsent, doctor can mutate/strip the web-search block while repairing the config — leaving Brave search without a usable key at runtime (the literal symptom in the #3948 title: "Brave API key is not usable inside sandbox"). Caught by CodeRabbit's review here.Fix
@openclaw/brave-plugin@$OPENCLAW_VERSIONwhenNEMOCLAW_WEB_SEARCH_ENABLEDis set, mirroring the messaging-channel and diagnostics-OTEL handling.BRAVE_API_KEYplaceholder into the doctor env when web search is enabled, the same way the channel bot tokens are already injected, so doctor preserves the resolvable web-search config.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_VERSIONand injects theBRAVE_API_KEYdoctor-env placeholder when web search is enabledOPENCLAW_VERSIONwhen web search is enabledVerified the
--dry-runoutput,--pininstall ordering, and doctor-env contents against each 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.comreturned200throughout — the API key and network policy were never the problem; only the plugin/host version skew and the build-time config repair.Fixes #3948
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Tests
Signed-off-by: John Ellison john@john-ellison.com