fix(onboard): write current web-search schema so the brave plugin install validates - #5318
Conversation
…tall validates Enabling Brave Web Search at onboarding still aborts the sandbox image build at the OpenClaw plugin-install step. `generate-openclaw-config.mts` writes the legacy `tools.web.search` shape with an inline `apiKey`, which OpenClaw 2026.5.x rejects. `openclaw plugins install` validates the existing config before installing, so it exits non-zero on the legacy shape while the brave plugin is not yet installed — aborting the build under `set -eu` before `openclaw doctor --fix` can migrate the schema. Emit the current schema directly: `tools.web.search` carries only `enabled` + `provider`, and the provider-owned apiKey placeholder moves to `plugins.entries.brave.config.webSearch`. This is exactly what `doctor --fix` produces after a successful install, so the build-time install validates cleanly (exit 0), doctor completes, and web search works alongside other channels (e.g. Slack) enabled in the same onboard. Follow-up to #4955 / #3948. Update the existing generate-config test to assert the current schema and the brave plugin entry. Fixes #5266 Signed-off-by: Zhi Yan Liu <lzy.dev@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
📝 WalkthroughWalkthroughGenerator now emits provider-only ChangesWeb Search Configuration Schema Migration
Sequence DiagramsequenceDiagram
participant Generator as buildConfig
participant OpenClawConfig as OpenClaw JSON
participant Verifier as verifyWebSearchInsideSandbox
participant Curl as egress probe (curl)
Generator->>OpenClawConfig: write tools.web.search { enabled, provider: "brave" }
Generator->>OpenClawConfig: write plugins.entries.brave.config.webSearch.apiKey (resolve BRAVE_API_KEY)
Verifier->>OpenClawConfig: read plugins.entries.brave.config.webSearch.apiKey
alt missing
Verifier->>OpenClawConfig: fallback read tools.web.search.apiKey
end
Verifier->>Curl: run probe using resolved apiKey
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/generate-openclaw-config.test.ts (1)
799-819:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffTest file size exceeds legacy budget (policy violation).
The test file has grown to 1998 lines, exceeding its legacy budget of 1990 lines. This is flagged as a policy failure in CI. While the added assertions are necessary to validate the new schema correctly, the test file is already at capacity.
Consider one of these remediation options:
- Extract some unrelated test cases into a separate test file to bring this file back under budget
- Request an exception to increase the budget for this file, given the critical nature of web-search configuration validation
🤖 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/generate-openclaw-config.test.ts` around lines 799 - 819, The test file test/generate-openclaw-config.test.ts exceeded the legacy size budget after adding assertions to the "enables web search when env is '1' using the current plugin schema" test (the block using runConfigScript and assertions on config.tools/web and config.plugins.entries.brave). Fix by splitting this file: move unrelated or older test cases out of generate-openclaw-config.test.ts into a new test file (keeping the new web-search test intact) and update any shared imports/fixtures so runConfigScript remains available; alternatively, if splitting is not desired, open a request to increase the file budget for generate-openclaw-config.test.ts so CI policy accepts the added assertions. Ensure the new file preserves the test suite structure and that the web-search test continues to assert config.tools?.web?.search, config.plugins?.entries?.brave, and related expectations.
🤖 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.
Inline comments:
In `@scripts/generate-openclaw-config.mts`:
- Around line 959-973: Update src/lib/onboard/web-search-verify.ts so the Brave
egress verifier looks up the apiKey from the new plugin config location as well
as the legacy inline location: check parsed.tools.web.search.apiKey (legacy) and
parsed.config?.plugins?.entries?.[provider]?.config?.webSearch?.apiKey (new
schema) where provider comes from parsed.tools.web.search.provider, and preserve
the existing validation that the apiKey is an openshell:resolve:env:*
placeholder; then update src/lib/onboard/web-search-verify.test.ts to build
fixtures using the new shape
(config.plugins.entries.brave.config.webSearch.apiKey) in tests and add a test
that verifies backward compatibility when tools.web.search.apiKey is present.
---
Outside diff comments:
In `@test/generate-openclaw-config.test.ts`:
- Around line 799-819: The test file test/generate-openclaw-config.test.ts
exceeded the legacy size budget after adding assertions to the "enables web
search when env is '1' using the current plugin schema" test (the block using
runConfigScript and assertions on config.tools/web and
config.plugins.entries.brave). Fix by splitting this file: move unrelated or
older test cases out of generate-openclaw-config.test.ts into a new test file
(keeping the new web-search test intact) and update any shared imports/fixtures
so runConfigScript remains available; alternatively, if splitting is not
desired, open a request to increase the file budget for
generate-openclaw-config.test.ts so CI policy accepts the added assertions.
Ensure the new file preserves the test suite structure and that the web-search
test continues to assert config.tools?.web?.search,
config.plugins?.entries?.brave, and related expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 78ca62dc-9a0f-43ae-9c0c-3af63396a48c
📒 Files selected for processing (2)
scripts/generate-openclaw-config.mtstest/generate-openclaw-config.test.ts
…g in the egress verifier Address CodeRabbit review on #5318: - web-search-verify.ts read the apiKey only from the legacy inline tools.web.search.apiKey location, so with the current plugin schema it warned "no API key placeholder" and silently skipped the Brave egress probe. Resolve the placeholder from plugins.entries.<provider>.config.webSearch.apiKey first and fall back to the legacy inline field so both schema generations stay probed. - Add a verifier test for the plugin-config location and keep a legacy inline-apiKey test for backward compatibility. - Tighten the new web-search assertions in test/generate-openclaw-config.test.ts (toEqual is already exact, so the separate apiKey-undefined assertion was redundant) to bring the file back under its legacy size budget, and ratchet the budget down to the new line count as the checker requires. Signed-off-by: Zhi Yan Liu <lzy.dev@gmail.com>
|
Addressed both CodeRabbit findings in 351beed:
Verified locally: budget check passes, |
wscurran
added
area: onboarding
|
✨ Thanks for updating the generate-openclaw-config script to emit the current web-search schema with the Brave provider config. This proposes a way to write the provider-owned apiKey placeholder to plugins.entries.brave.config.webSearch so the build-time plugin install validates cleanly. Related open PRs: Related open issues:
|
Selective E2E Results — ✅ All requested jobs passedRun: 27458771824
|
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
CI is green on the current head. I verified the CodeRabbit web-search schema and file-size findings were addressed: the verifier supports the plugin config apiKey path with legacy fallback, generate-openclaw-config stays within the ratcheted budget, and the targeted brave-search-e2e run 27458771824 passed for head 93605c7.
3 participants
Summary
Enabling Brave Web Search at onboarding still aborts the sandbox image build at the OpenClaw plugin-install step.
scripts/generate-openclaw-config.mtswrites the legacytools.web.searchshape (inlineapiKey), which OpenClaw 2026.5.x rejects — and becauseopenclaw plugins installvalidates the existing config before installing, it exits non-zero while the brave plugin is not yet installed, aborting the build underset -eubeforeopenclaw doctor --fixcan 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: whenNEMOCLAW_WEB_SEARCH_ENABLED=1, writetools.web.search = { enabled, provider: "brave" }and move the provider-ownedapiKeyplaceholder toplugins.entries.brave.config.webSearch— the same shapedoctor --fixproduces after a successful install.test/generate-openclaw-config.test.ts: update the existing web-search case to assert the current schema (no inlineapiKey) and theplugins.entries.braveentry.Why this is the fix
#4955pinned/installed@openclaw/brave-pluginat build time and threadedBRAVE_API_KEYinto the doctor env, assumingdoctor --fixwould migrate the legacy block. Butplugins installruns 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):End-to-end: a real
nemoclaw onboard --recreate-sandboxwith Brave Web Search and Slack both enabled now builds cleanly, andweb_searchworks in the running sandbox alongside Slack.Type of Change
Verification
test/generate-openclaw-config.test.tspasses (128/128) — covers the changed codenpm run typecheck:clipasses;npx biome lintclean on changed filesnpm 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
Summary by CodeRabbit
Chores
Tests
Bug Fixes