feat(onboard): add Brave Search onboarding via BRAVE_API_KEY#1464
Merged
feat(onboard): add Brave Search onboarding via BRAVE_API_KEY#1464
Conversation
Contributor
📝 WalkthroughWalkthroughAdds Brave Web Search integration: Docker build argument propagation, base64 web-config encoding/decoding, onboarding prompts and session persistence for web search, sandbox Dockerfile injection, gateway config merge for Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as Onboarding CLI
participant Session as Session Manager
participant WebSearch as WebSearch Module
participant DockerPatcher as Dockerfile Patcher
participant Sandbox as Sandbox Manager
participant Gateway as Gateway Runtime
User->>CLI: run onboarding (interactive or env BRAVE_API_KEY)
CLI->>Session: createSession(overrides including webSearch)
Session->>Session: validate/normalize webSearchConfig
Session-->>CLI: webSearchConfig or null
CLI->>WebSearch: buildWebSearchDockerConfig(webSearchConfig, BRAVE_API_KEY)
WebSearch->>WebSearch: serialize JSON, Base64-encode
WebSearch-->>CLI: NEMOCLAW_WEB_CONFIG_B64
CLI->>DockerPatcher: patchStagedDockerfile(..., NEMOCLAW_WEB_CONFIG_B64)
DockerPatcher->>DockerPatcher: inject ARG/ENV into Dockerfile
DockerPatcher-->>CLI: patched Dockerfile
CLI->>Sandbox: createSandbox(..., webSearchConfig)
Sandbox->>Sandbox: validate BRAVE_API_KEY presence if enabled
Sandbox->>Gateway: start container image
Gateway->>Gateway: decode NEMOCLAW_WEB_CONFIG_B64
Gateway->>Gateway: merge tools.web when provider == "brave"
Gateway-->>User: sandbox ready (with Brave web search configured)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
kjw3
changed the title
cv
approved these changes
Apr 4, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard-session.ts (1)
79-89:⚠️ Potential issue | 🟡 Minor
webSearchConfigis still dropped by the safe-update path.
SessionUpdatesnow advertiseswebSearchConfig, butfilterSafeUpdates()never copies it into the persisted session. Any caller that usesmarkStepComplete()orcompleteSession()with this field will silently lose it, while direct mutators behave differently. Please either wire it through there or remove it fromSessionUpdatesuntil the helper supports it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard-session.ts` around lines 79 - 89, SessionUpdates declares webSearchConfig but filterSafeUpdates does not copy it, causing callers like markStepComplete and completeSession to silently drop webSearchConfig; update filterSafeUpdates to include webSearchConfig in the set of allowed/whitelisted fields copied into the persisted session (mirror how policyPresets/metadata are handled) so any caller using markStepComplete/completeSession preserves webSearchConfig, or alternatively remove webSearchConfig from SessionUpdates if you prefer not to support it via filterSafeUpdates yet.
🧹 Nitpick comments (2)
docs/reference/commands.md (1)
76-76: Prefer present tense in the Brave credential-path sentence.Consider rephrasing “explored” into present-tense wording to match docs voice rules.
As per coding guidelines: “Present tense.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/commands.md` at line 76, The sentence "NemoClaw explored an OpenShell-hosted credential path first, but the current OpenClaw Brave runtime does not consume that path end to end yet." uses past tense; change "explored" to present tense (for example, "explores") so it reads "NemoClaw explores an OpenShell-hosted credential path first, but the current OpenClaw Brave runtime does not consume that path end to end yet." to match the docs' present-tense voice.src/lib/web-search.test.ts (1)
12-25: Add a regression test for{ fetchEnabled: false }.This closes the branch where config exists but web search should still stay disabled.
Suggested test case
describe("web-search helpers", () => { it("emits empty docker config when web search is disabled", () => { expect(Buffer.from(buildWebSearchDockerConfig(null, null), "base64").toString("utf8")).toBe( "{}", ); }); + + it("emits empty docker config when fetchEnabled is false", () => { + expect( + Buffer.from( + buildWebSearchDockerConfig({ fetchEnabled: false }, "brv-x"), + "base64", + ).toString("utf8"), + ).toBe("{}"); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/web-search.test.ts` around lines 12 - 25, Add a test to ensure buildWebSearchDockerConfig treats an explicit { fetchEnabled: false } as disabled: call buildWebSearchDockerConfig({ fetchEnabled: false }, null) (or empty api key), base64-decode the result and assert the UTF-8 string equals "{}" (same expectation as the existing "web search is disabled" test) to cover the branch where a config object exists but web search remains off; use the test name like "emits empty docker config when fetchEnabled is false" and reference the buildWebSearchDockerConfig function in the new spec.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin/lib/onboard.js`:
- Around line 4030-4038: When webSearchConfig exists, do not reuse it blindly:
rehydrate and revalidate the Brave credential by calling
ensureValidatedBraveSearchCredential() (or equivalent) before treating
webSearchConfig as reusable; if validation fails or returns a refreshed key,
update webSearchConfig accordingly and call
onboardSession.updateSession((c)=>{c.webSearchConfig = webSearchConfig; return
c}); otherwise fall through to configureWebSearch(webSearchConfig) to obtain a
new config; ensure createSandbox() never receives a stale BRAVE_API_KEY by
performing this check in the branch that currently logs "[resume] Reusing Brave
Search configuration."
- Around line 2347-2351: runOpenshell is being called with suppressOutput: true
which hides all stdout/stderr on failure; change the call to capture its return
value (from runOpenshell(["gateway","start","--name", GATEWAY_NAME], {
ignoreError: true, env: getGatewayStartEnv(), suppressOutput: true })) then
inspect the returned exit code and on non-zero produce a trimmed, redacted
summary of stdout/stderr to the logger (e.g., processLogger.warn/error) so
successes remain quiet but failures emit a concise diagnostic; keep using
runOpenshell, getGatewayStartEnv and GATEWAY_NAME to locate the call and ensure
any output is sanitized/trimmed before logging.
In `@Dockerfile`:
- Around line 125-138: The code is writing the raw Brave API key into the
sandboxed config via config.update(...) (setting tools.web.search.apiKey from
web_config.get('apiKey')), which exposes the secret; instead remove embedding
the raw key into tools.web.search.apiKey and pass a host-side secret reference
or proxy token (e.g., set tools.web.search.secretRef or
tools.web.search.proxyEndpoint) so the sandbox only receives a reference; update
the logic around config.update and the web_config.get('apiKey') usage to
populate a non-secret identifier (secretRef/proxy URL) and ensure any code that
uses tools.web.search.apiKey is changed to resolve the secret from the host-side
secret store or call the proxied endpoint.
- Line 57: Replace the ARG-only declaration for NEMOCLAW_WEB_CONFIG_B64 with an
ENV declaration and stop performing inline shell expansion in RUN lines that use
it: keep ARG NEMOCLAW_WEB_CONFIG_B64 (if you need build-time default) and add
ENV NEMOCLAW_WEB_CONFIG_B64=${NEMOCLAW_WEB_CONFIG_B64}, then update the RUN
commands (the commands around the current RUN usage at lines ~85-95) to
reference the environment variable directly (e.g. use $NEMOCLAW_WEB_CONFIG_B64
or ${NEMOCLAW_WEB_CONFIG_B64} without embedding it into a quoted assignment like
NEMOCLAW_WEB_CONFIG_B64="${...}") so the value isn’t subject to shell command
evaluation during build.
In `@src/lib/web-search.ts`:
- Around line 25-31: The payload currently hardcodes fetchEnabled: true which
ignores the incoming config; update the payload construction in
src/lib/web-search.ts to set fetchEnabled based on config.fetchEnabled (e.g.,
use Boolean(config.fetchEnabled) or default to false when undefined) so the web
search respects the caller's config; keep provider: "brave" and apiKey:
braveApiKey || "" as-is and only change the fetchEnabled value in the payload
object.
In `@test/onboard.test.js`:
- Around line 243-265: Save the prior value of process.env.BRAVE_API_KEY at the
start of the test and restore it in the finally block instead of unconditionally
deleting it; specifically, before setting process.env.BRAVE_API_KEY =
"brv-test-key" capture const priorBraveKey = process.env.BRAVE_API_KEY, and in
the finally block set process.env.BRAVE_API_KEY = priorBraveKey (or delete it
only if priorBraveKey is undefined/null) so the test's use of
patchStagedDockerfile and assertions do not leak or clobber existing environment
state.
---
Outside diff comments:
In `@src/lib/onboard-session.ts`:
- Around line 79-89: SessionUpdates declares webSearchConfig but
filterSafeUpdates does not copy it, causing callers like markStepComplete and
completeSession to silently drop webSearchConfig; update filterSafeUpdates to
include webSearchConfig in the set of allowed/whitelisted fields copied into the
persisted session (mirror how policyPresets/metadata are handled) so any caller
using markStepComplete/completeSession preserves webSearchConfig, or
alternatively remove webSearchConfig from SessionUpdates if you prefer not to
support it via filterSafeUpdates yet.
---
Nitpick comments:
In `@docs/reference/commands.md`:
- Line 76: The sentence "NemoClaw explored an OpenShell-hosted credential path
first, but the current OpenClaw Brave runtime does not consume that path end to
end yet." uses past tense; change "explored" to present tense (for example,
"explores") so it reads "NemoClaw explores an OpenShell-hosted credential path
first, but the current OpenClaw Brave runtime does not consume that path end to
end yet." to match the docs' present-tense voice.
In `@src/lib/web-search.test.ts`:
- Around line 12-25: Add a test to ensure buildWebSearchDockerConfig treats an
explicit { fetchEnabled: false } as disabled: call buildWebSearchDockerConfig({
fetchEnabled: false }, null) (or empty api key), base64-decode the result and
assert the UTF-8 string equals "{}" (same expectation as the existing "web
search is disabled" test) to cover the branch where a config object exists but
web search remains off; use the test name like "emits empty docker config when
fetchEnabled is false" and reference the buildWebSearchDockerConfig function in
the new spec.
🪄 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: Pro
Run ID: 3f6923ec-c7e7-4ef0-8516-7e27d90cf6ec
📒 Files selected for processing (11)
Dockerfilebin/lib/onboard.jsbin/lib/runner.jsdocs/reference/commands.mdnemoclaw-blueprint/policies/presets/brave.yamlscripts/install.shsrc/lib/onboard-session.tssrc/lib/web-search.test.tssrc/lib/web-search.tstest/onboard.test.jstest/policies.test.js
Comment on lines
+25
to
+31
| if (!config) return encodeDockerJsonArg({}); | ||
|
|
||
| const payload = { | ||
| provider: "brave", | ||
| fetchEnabled: true, | ||
| apiKey: braveApiKey || "", | ||
| }; |
Contributor
There was a problem hiding this comment.
Respect config.fetchEnabled instead of forcing true.
Current logic enables web config whenever config is non-null, even if fetchEnabled is false.
Suggested fix
export function buildWebSearchDockerConfig(
config: WebSearchConfig | null,
braveApiKey: string | null,
): string {
- if (!config) return encodeDockerJsonArg({});
+ if (!config || !config.fetchEnabled) return encodeDockerJsonArg({});
const payload = {
provider: "brave",
- fetchEnabled: true,
- apiKey: braveApiKey || "",
+ fetchEnabled: config.fetchEnabled,
+ apiKey: braveApiKey ?? "",
};
return encodeDockerJsonArg(payload);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!config) return encodeDockerJsonArg({}); | |
| const payload = { | |
| provider: "brave", | |
| fetchEnabled: true, | |
| apiKey: braveApiKey || "", | |
| }; | |
| if (!config || !config.fetchEnabled) return encodeDockerJsonArg({}); | |
| const payload = { | |
| provider: "brave", | |
| fetchEnabled: config.fetchEnabled, | |
| apiKey: braveApiKey ?? "", | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/web-search.ts` around lines 25 - 31, The payload currently hardcodes
fetchEnabled: true which ignores the incoming config; update the payload
construction in src/lib/web-search.ts to set fetchEnabled based on
config.fetchEnabled (e.g., use Boolean(config.fetchEnabled) or default to false
when undefined) so the web search respects the caller's config; keep provider:
"brave" and apiKey: braveApiKey || "" as-is and only change the fetchEnabled
value in the payload object.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/onboard-session.ts (1)
58-58: IncludewebSearchConfigin debug summaries for onboarding diagnostics.Now that this field is persisted, omitting it from
summarizeForDebug()makes troubleshooting harder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard-session.ts` at line 58, summarizeForDebug() currently omits the persisted webSearchConfig field; update the method in the OnboardSession (or whichever class defines summarizeForDebug) to include webSearchConfig in its returned debug summary (or string), handling null/undefined safely (e.g., include "none" or JSON.stringify/webSearchConfig as appropriate) and reference the existing property name webSearchConfig so the debug output reflects its current value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/onboard-session.ts`:
- Line 87: The session update path has a mismatch:
SessionUpdates.webSearchConfig can carry { fetchEnabled: false } to disable web
search but filterSafeUpdates() currently ignores that shape and leaves an
enabled session unchanged; update filterSafeUpdates (and any related validation
in onboard-session.ts) to recognize webSearchConfig presence even when
fetchEnabled is false and apply that update to the session (i.e., overwrite or
set webSearchConfig.fetchEnabled = false on the existing session). Specifically,
adjust logic that checks for webSearchConfig to treat a non-null object with
fetchEnabled:false as an explicit update (not as absent), handling the same for
the other occurrence referenced around lines 416-420 so toggling off works as
intended.
---
Nitpick comments:
In `@src/lib/onboard-session.ts`:
- Line 58: summarizeForDebug() currently omits the persisted webSearchConfig
field; update the method in the OnboardSession (or whichever class defines
summarizeForDebug) to include webSearchConfig in its returned debug summary (or
string), handling null/undefined safely (e.g., include "none" or
JSON.stringify/webSearchConfig as appropriate) and reference the existing
property name webSearchConfig so the debug output reflects its current value.
🪄 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: Pro
Run ID: 06e55a13-849e-40d2-9319-c35e6ea7684f
📒 Files selected for processing (8)
Dockerfilebin/lib/onboard.jsdocs/reference/commands.mdsrc/lib/onboard-session.tssrc/lib/web-search.test.tssrc/lib/web-search.tstest/onboard.test.jstest/usage-notice.test.js
✅ Files skipped from review due to trivial changes (2)
- src/lib/web-search.test.ts
- docs/reference/commands.md
🚧 Files skipped from review as they are similar to previous changes (3)
- Dockerfile
- src/lib/web-search.ts
- bin/lib/onboard.js
| credentialEnv?: string; | ||
| preferredInferenceApi?: string; | ||
| nimContainer?: string; | ||
| webSearchConfig?: WebSearchConfig | null; |
Contributor
There was a problem hiding this comment.
webSearchConfig boolean contract is inconsistent in update flow.
SessionUpdates.webSearchConfig allows fetchEnabled: false, but filterSafeUpdates() ignores that shape instead of disabling config. If a session is already enabled, passing { fetchEnabled: false } leaves it enabled.
💡 Proposed fix
export function filterSafeUpdates(updates: SessionUpdates): Partial<Session> {
const safe: Partial<Session> = {};
if (!isObject(updates)) return safe;
@@
- if (isObject(updates.webSearchConfig) && updates.webSearchConfig.fetchEnabled === true) {
- safe.webSearchConfig = { fetchEnabled: true };
- } else if (updates.webSearchConfig === null) {
- safe.webSearchConfig = null;
- }
+ if (isObject(updates.webSearchConfig)) {
+ if (updates.webSearchConfig.fetchEnabled === true) {
+ safe.webSearchConfig = { fetchEnabled: true };
+ } else if (updates.webSearchConfig.fetchEnabled === false) {
+ safe.webSearchConfig = null;
+ }
+ } else if (updates.webSearchConfig === null) {
+ safe.webSearchConfig = null;
+ }Also applies to: 416-420
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/onboard-session.ts` at line 87, The session update path has a
mismatch: SessionUpdates.webSearchConfig can carry { fetchEnabled: false } to
disable web search but filterSafeUpdates() currently ignores that shape and
leaves an enabled session unchanged; update filterSafeUpdates (and any related
validation in onboard-session.ts) to recognize webSearchConfig presence even
when fetchEnabled is false and apply that update to the session (i.e., overwrite
or set webSearchConfig.fetchEnabled = false on the existing session).
Specifically, adjust logic that checks for webSearchConfig to treat a non-null
object with fetchEnabled:false as an explicit update (not as absent), handling
the same for the other occurrence referenced around lines 416-420 so toggling
off works as intended.
wscurran
added
Getting Started
7 tasks
gemini2026
pushed a commit
to gemini2026/NemoClaw
that referenced
this pull request
Apr 14, 2026
…1464) ## Summary - add NemoClaw onboarding support for Brave web search via `BRAVE_API_KEY` - validate the Brave API key during onboarding and keep retry prompts masked - add a Brave policy preset and bake Brave web config into immutable sandbox `openclaw.json` ## Issues - addresses NVIDIA#773 by adding a supported NemoClaw path to enable `web_search` without relying on `openclaw configure --section web` - addresses NVIDIA#988 by moving Brave web-search setup into onboarding instead of mutable in-sandbox provider switching - addresses NVIDIA#518 for the Brave path by baking the Brave web config into the sandbox image instead of relying on post-start config inheritance - partially addresses NVIDIA#719 for web-search onboarding by providing a first-class setup path despite immutable `openclaw.json` - informed by NVIDIA#396, NVIDIA#1063, and NVIDIA#1252, but does not fully solve the upstream trusted env-proxy DNS/runtime issues for all search providers ## Notes - this implementation enables `web_search` and `web_fetch` in OpenClaw config when `BRAVE_API_KEY` is set - policy still controls which destinations are actually reachable from the sandbox - the Brave API key is exposed to the OpenClaw agent because it is stored in sandbox-readable OpenClaw config ## Why this shape - we explored using an OpenShell-hosted Brave credential first - OpenShell can host `BRAVE_API_KEY`, but current OpenClaw Brave handling does not consume the OpenShell secret-ref path end to end - this should be improved in the future so the Brave key does not need to be exposed in sandbox config ## Verification - `npm run build:cli` - `npm run typecheck:cli` - `npm test -- src/lib/web-search.test.ts test/onboard.test.js test/policies.test.js` - `npm test -- test/onboard.test.js` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Brave Web Search integration in onboarding and sandbox creation; opt-in enables web search and optional fetch behavior. * Non-interactive setup can enable Brave Search via BRAVE_API_KEY environment variable. * **Documentation** * Added guidance and warnings about Brave API key usage and non-interactive examples. * **Chores** * Added a Brave policy preset and updated tests/fixtures to reflect web search configuration. * **User Experience** * Improved sandbox build/startup log formatting and progress output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Aaron Erickson 🦞 <aerickson@nvidia.com>
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
6 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
BRAVE_API_KEYopenclaw.jsonIssues
web_searchwithout relying onopenclaw configure --section webopenclaw.jsonNotes
web_searchandweb_fetchin OpenClaw config whenBRAVE_API_KEYis setWhy this shape
BRAVE_API_KEY, but current OpenClaw Brave handling does not consume the OpenShell secret-ref path end to endVerification
npm run build:clinpm run typecheck:clinpm test -- src/lib/web-search.test.ts test/onboard.test.js test/policies.test.jsnpm test -- test/onboard.test.jsSummary by CodeRabbit
New Features
Documentation
Chores
User Experience