feat(web-search): add DuckDuckGo provider gated behind experimental#4674
Open
laitingsheng wants to merge 10 commits into
Open
feat(web-search): add DuckDuckGo provider gated behind experimental#4674laitingsheng wants to merge 10 commits into
laitingsheng wants to merge 10 commits into
Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
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 (11)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds DuckDuckGo as a selectable, keyless web-search provider and propagates provider choice through Docker/ENV, generated OpenClaw config, onboarding/session handling, policy presets and staleness logic, sandbox verification, provider/preset YAMLs, tests, docs, and nightly E2E CI. ChangesDuckDuckGo Web Search Provider
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorFailed: Could not parse JSON from advisor output; see /home/runner/work/NemoClaw/NemoClaw/artifacts/e2e-advisor/e2e-advisor-raw-output.txt |
Contributor
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorFailed: Could not parse JSON from advisor output; see /home/runner/work/NemoClaw/NemoClaw/artifacts/e2e-advisor/e2e-scenario-advisor-raw-output.txt |
Contributor
PR Review AdvisorFindings: 3 needs attention, 4 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
…ent DDG Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-4674.docs.buildwithfern.com/nemoclaw |
![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: 3
🧹 Nitpick comments (1)
src/lib/onboard/web-search-flow.ts (1)
245-248: 💤 Low valueRemove unreachable null check.
Since
promptWebSearchProvider()never returnsnull(it returns"brave"or"duckduckgo", or exits), andduckDuckGoRequestedis boolean,providercannot benullat line 246. Based on learnings, avoid defensive handling when there is no realistic failure mode.♻️ Proposed simplification
const provider = duckDuckGoRequested ? "duckduckgo" : await promptWebSearchProvider(); - if (provider === null) { - return null; - } if (provider === "duckduckgo") {🤖 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 `@src/lib/onboard/web-search-flow.ts` around lines 245 - 248, The null check for provider is unreachable because provider is assigned from duckDuckGoRequested (boolean) or promptWebSearchProvider() which never returns null; remove the defensive branch that returns null. Specifically, delete the if (provider === null) { return null; } block in the web-search flow so callers can assume provider is a valid string (from promptWebSearchProvider or "duckduckgo"); ensure any downstream code relies on provider as a non-null string and adjust types if necessary.
🤖 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 `@docs/reference/commands.mdx`:
- Around line 238-239: Update the non-interactive onboarding example so it
explicitly accepts third-party software; add the flag
--yes-i-accept-third-party-software (or set
NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1) to the command shown (the example with
NEMOCLAW_EXPERIMENTAL and NEMOCLAW_WEB_SEARCH_PROVIDER=duckduckgo that runs
`nemoclaw onboard --non-interactive`) so the copy-pasteable example succeeds
without manual interaction.
In `@nemoclaw-blueprint/policies/presets/duckduckgo.yaml`:
- Around line 24-27: Mismatch in the binaries allowlist: the duckduckgo preset's
binaries block only lists "/usr/bin/curl" while the provider profile allows both
"/usr/local/bin/curl" and "/usr/bin/curl"; update the binaries array in the
presets file (the "binaries" entry and its curl item) to include the same curl
paths as the provider profile so egress checks won't be blocked (i.e., add the
"/usr/local/bin/curl" entry alongside "/usr/bin/curl").
In `@src/lib/onboard/web-search-flow.ts`:
- Around line 175-190: The function promptWebSearchProvider never returns null,
so update its signature to return Promise<WebSearchProvider> instead of
Promise<WebSearchProvider | null>; modify the declaration of
promptWebSearchProvider accordingly and ensure any call sites expecting null are
adjusted (there should be none since the function either returns
"brave"/"duckduckgo" or exits via exitOnboardFromPrompt); keep the internal
logic returning "brave" or "duckduckgo" unchanged.
---
Nitpick comments:
In `@src/lib/onboard/web-search-flow.ts`:
- Around line 245-248: The null check for provider is unreachable because
provider is assigned from duckDuckGoRequested (boolean) or
promptWebSearchProvider() which never returns null; remove the defensive branch
that returns null. Specifically, delete the if (provider === null) { return
null; } block in the web-search flow so callers can assume provider is a valid
string (from promptWebSearchProvider or "duckduckgo"); ensure any downstream
code relies on provider as a non-null string and adjust types if necessary.
🪄 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: 1a061b8d-8478-41e1-8fe6-77d1528e96a3
📒 Files selected for processing (18)
Dockerfiledocs/reference/commands.mdxnemoclaw-blueprint/policies/presets/duckduckgo.yamlnemoclaw-blueprint/provider-profiles/duckduckgo.yamlscripts/generate-openclaw-config.mtssrc/lib/inference/web-search.tssrc/lib/onboard.tssrc/lib/onboard/brave-provider-profile.tssrc/lib/onboard/dockerfile-patch.tssrc/lib/onboard/policy-presets.tssrc/lib/onboard/policy-selection.tssrc/lib/onboard/web-search-flow.tssrc/lib/onboard/web-search-verify.tssrc/lib/policy/index.tssrc/lib/state/onboard-session.tstest/e2e/test-duckduckgo-search-e2e.shtest/onboard-brave-validation.test.tstest/policies.test.ts
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26860420350
|
wscurran
added
area: providers
…ig, helper Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26861505717
|
…per, refresh docs Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/onboard/policy-selection.ts (1)
201-205: 💤 Low valueLocal alias
isStaleBuiltinBraveis now misleading.The helper is provider-aware (handles
duckduckgotoo), so the local name reads as Brave-only. Optional rename toisStaleBuiltinWebSearchfor clarity; behavior is correct as-is.Also applies to: 292-293
🤖 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 `@src/lib/onboard/policy-selection.ts` around lines 201 - 205, The local alias isStaleBuiltinBrave is misleading because the underlying helper is provider-aware; rename the alias to isStaleBuiltinWebSearch to reflect that it covers Brave and DuckDuckGo and update all usages (the alias definition that calls isStaleBuiltinWebSearchPolicyPreset and the later reference at the other occurrence) to the new name; keep the call arguments (webSearchConfig and customPolicyPresetNames) unchanged so behavior stays the same.
🤖 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 `@docs/reference/network-policies.mdx`:
- Line 75: Replace the passive sentence "The unused web-search preset is pruned
from the tier defaults." with an active-voice version; update the text to
"NemoClaw prunes the unused web-search preset from the tier defaults." so the
docs follow the active-voice guideline for docs/**.
---
Nitpick comments:
In `@src/lib/onboard/policy-selection.ts`:
- Around line 201-205: The local alias isStaleBuiltinBrave is misleading because
the underlying helper is provider-aware; rename the alias to
isStaleBuiltinWebSearch to reflect that it covers Brave and DuckDuckGo and
update all usages (the alias definition that calls
isStaleBuiltinWebSearchPolicyPreset and the later reference at the other
occurrence) to the new name; keep the call arguments (webSearchConfig and
customPolicyPresetNames) unchanged so behavior stays the same.
🪄 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: 28e6d8ba-7ee5-48b4-a95d-a4197557a542
📒 Files selected for processing (8)
.github/workflows/nightly-e2e.yamldocs/get-started/quickstart.mdxdocs/reference/network-policies.mdxsrc/lib/onboard/policy-selection.tssrc/lib/onboard/web-search-verify.test.tssrc/lib/onboard/web-search-verify.tssrc/lib/state/onboard-session.tstest/onboard-policy-suggestions.test.ts
✅ Files skipped from review due to trivial changes (1)
- docs/get-started/quickstart.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/onboard/web-search-verify.ts
- src/lib/state/onboard-session.ts
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26863024417
|
…e voice, rename local alias Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26864205349
|
…interactive too Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26865473413
|
…web-search Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26887302061
|
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…onflicts with latest main Signed-off-by: Tinson Lai <tinsonl@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
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
Adds a keyless DuckDuckGo Web Search provider as an experimental alternative to Brave Search. Activation is gated by
NEMOCLAW_EXPERIMENTAL=1in non-interactive mode and by an additional provider prompt in interactive mode; without that flag, the existing Brave-only behavior is preserved.Related Issue
Resolves #4559.
Changes
WebSearchConfiggains an optionalproviderdiscriminant (brave|duckduckgo); onboard-session serialization preserves it.scripts/generate-openclaw-config.mtsbranches onNEMOCLAW_WEB_SEARCH_PROVIDER: DuckDuckGo emitstools.web.search.provider = "duckduckgo"with noapiKeyfield; Brave keeps its OpenShell placeholder.DockerfiledeclaresNEMOCLAW_WEB_SEARCH_PROVIDERandsrc/lib/onboard/dockerfile-patch.tsthreads it from the onboard config.src/lib/onboard/web-search-flow.tsconfigureWebSearchadds a DuckDuckGo path that bypasses the Brave API-key flow entirely.src/lib/onboard/brave-provider-profile.tsno longer treats a non-Brave provider as Brave-active.src/lib/onboard/web-search-verify.tsrecognizes the DuckDuckGo branch.nemoclaw-blueprint/provider-profiles/duckduckgo.yamlandnemoclaw-blueprint/policies/presets/duckduckgo.yamlallow GET/POST onduckduckgo.comandlite.duckduckgo.comfrom the samenode/curlbinaries Brave whitelists.test/e2e/test-duckduckgo-search-e2e.shmirrors the Brave suite minus the secret-handling phases and asserts that noapiKey/api_keyfield is emitted.Type of Change
Verification
Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
Policy
Documentation
Tests / CI