fix(onboard): drop stale disabled web-search providers by HOYALIM · Pull Request #6531 · NVIDIA/NemoClaw

HOYALIM · 2026-07-09T01:36:07Z

Summary

treat webSearchConfig: null as an authoritative managed web-search disable when preparing sandbox create providers
filter stale brave-search / tavily-search extra providers while preserving unrelated custom extra providers
update sandbox create coverage so a stale tavily-search registry entry no longer reaches openshell sandbox create when web search is disabled

Tests

npm run build:cli
npx vitest run src/lib/onboard/sandbox-create-plan.test.ts test/onboard.test.ts
npx vitest run test/onboard.test.ts -t "builds the sandbox without uploading an external OpenClaw config file"
npm run check:diff

DCO

I certify that this contribution is submitted under the Developer Certificate of Origin and the commit is signed off with Signed-off-by.

Closes #6501

Summary by CodeRabbit

Bug Fixes
- Improved sandbox plan generation to correctly honor web search configuration.
- When web search is explicitly disabled, web-search-related providers are excluded from generated provider flags.
- Provider selection for web search is now derived from the chosen configuration.
Tests
- Added coverage for selecting the managed web-search provider while excluding other web-search providers.
- Tightened sandbox creation command assertions to ensure only the expected provider flags are present.

copy-pr-bot · 2026-07-09T01:36:10Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

coderabbitai · 2026-07-09T01:36:40Z

📝 Walkthrough

Walkthrough

Adds webSearchConfig-aware extra-provider filtering to sandbox plan creation, so managed web-search providers are removed when web search is explicitly disabled. The onboard flow now passes that configuration through, and tests verify the resulting command arguments.

Changes

Web search disabled provider filtering

Layer / File(s)	Summary
normalizeExtraProvidersForWebSearch contract and implementation `src/lib/onboard/sandbox-create-plan.ts`	Adds `WEB_SEARCH_PROVIDERS` import, `MANAGED_WEB_SEARCH_EXTRA_PROVIDERS` constant, optional `webSearchConfig` on `PrepareSandboxCreatePlanInput`, and `normalizeExtraProvidersForWebSearch` to dedupe/filter `extraProviders`, removing managed web-search providers when `webSearchConfig` is `null`.
Wire webSearchConfig into prepareSandboxCreatePlan and onboard flow `src/lib/onboard/sandbox-create-plan.ts`, `src/lib/onboard.ts`	`prepareSandboxCreatePlan` destructures and uses `webSearchConfig` when resolving the sandbox create intent; `onboard.ts` passes `webSearchConfig` into `prepareSandboxCreatePlan`.
Tests verifying stale web-search providers are excluded `src/lib/onboard/sandbox-create-plan.test.ts`, `test/onboard.test.ts`	New test confirms `brave-search` and `tavily-search` are excluded from `createArgs` when `webSearchConfig` is `null`; the onboard test now asserts `--provider tavily-search` is absent.

Estimated code review effort: 2 (Simple) | ~12 minutes

Suggested labels: bug-fix

Suggested reviewers: kjw3, ericksoa

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: removing stale web-search providers during onboard sandbox creation.
Linked Issues check	✅ Passed	The code filters managed web-search providers when webSearchConfig is null, preventing stale tavily-search references as requested.
Out of Scope Changes check	✅ Passed	All changes stay focused on onboard sandbox provider selection and its tests; no unrelated behavior was introduced.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 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 `@src/lib/onboard/sandbox-create-plan.ts`:
- Around line 160-167: `normalizeExtraProvidersForWebSearch` only removes
managed web-search providers when web search is disabled, so stale entries can
survive provider switches. Update this helper to also filter out managed
providers that do not match `webSearchConfig.provider` when a web search config
is present, keeping only the selected provider’s managed extra provider and
discarding old state. Use `normalizeExtraProvidersForWebSearch`,
`MANAGED_WEB_SEARCH_EXTRA_PROVIDERS`, and `webSearchConfig.provider` to locate
the change.

🪄 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: fdd24d8e-9630-4ef0-83bf-e49feddba6be

📥 Commits

Reviewing files that changed from the base of the PR and between 17bf9a6 and bb37e07.

📒 Files selected for processing (4)

src/lib/onboard.ts
src/lib/onboard/sandbox-create-plan.test.ts
src/lib/onboard/sandbox-create-plan.ts
test/onboard.test.ts

Signed-off-by: Ho Lim <subhoya@gmail.com>

coderabbitai

🧹 Nitpick comments (1)

src/lib/onboard/sandbox-create-plan.test.ts (1)
592-631: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add explicit coverage for webSearchConfig: null (the actual bug scenario).

This test covers the "provider selected" filtering branch (brave selected, tavily/brave-search filtered per selection). But the linked issue's actual failure mode is webSearchConfig: null (web search disabled entirely), where normalizeExtraProvidersForWebSearch should strip both brave-search and tavily-search while keeping custom-provider. Adding a sibling test case asserting providerArgs excludes both managed providers when webSearchConfig is null would directly pin down the regression this PR fixes at the unit level, rather than relying only on the higher-level test/onboard.test.ts assertion.

Want me to draft the additional webSearchConfig: null test case?
🤖 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/sandbox-create-plan.test.ts` around lines 592 - 631, Add a
sibling unit test in prepareSandboxCreatePlan coverage for the actual
disabled-web-search case by setting webSearchConfig to null and asserting
normalizeExtraProvidersForWebSearch removes both managed providers. Reuse
prepareSandboxCreatePlan and inspect result.createArgs/providerArgs the same way
as the existing test, but verify only custom-provider remains when
webSearchConfig is null.

🤖 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 `@src/lib/onboard/sandbox-create-plan.test.ts`:
- Around line 592-631: Add a sibling unit test in prepareSandboxCreatePlan
coverage for the actual disabled-web-search case by setting webSearchConfig to
null and asserting normalizeExtraProvidersForWebSearch removes both managed
providers. Reuse prepareSandboxCreatePlan and inspect
result.createArgs/providerArgs the same way as the existing test, but verify
only custom-provider remains when webSearchConfig is null.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b9480918-0607-424e-bb61-5091a6fff5c0

📥 Commits

Reviewing files that changed from the base of the PR and between e270203 and 81556b1.

📒 Files selected for processing (4)

src/lib/onboard.ts
src/lib/onboard/sandbox-create-plan.test.ts
src/lib/onboard/sandbox-create-plan.ts
test/onboard.test.ts

🚧 Files skipped from review as they are similar to previous changes (3)

src/lib/onboard.ts
test/onboard.test.ts
src/lib/onboard/sandbox-create-plan.ts

cjagwani

Two blockers remain on exact head 81556b12a7ccfd84704973a7392d11df641a67b2.

The new name-based ownership rule is incorrect and regresses the documented Deep Agents flow. Bare tavily-search is a user-registered, gateway-wide extra provider created by nemoclaw credentials add tavily-search ...; it is recorded in registry.listExtraProviders() and intentionally attaches to every later build/rebuild. Managed OpenClaw/Hermes web-search providers are instead sandbox-scoped (<sandbox>-brave-search / <sandbox>-tavily-search) and travel through the messaging/provider preparation path. Because Deep Agents has webSearchConfig: null, normalizeExtraProvidersForWebSearch() now silently drops exactly its documented tavily-search credential provider. Conversely, if Tavily is selected, a stale missing bare tavily-search remains and can still reproduce the original create failure.

Please preserve healthy user-owned extra providers and reconcile stale state by provider existence or explicit provenance rather than reserving bare names. Add regression coverage that (a) a valid gateway-wide tavily-search extra provider still attaches for Deep Agents / disabled managed web search and (b) a missing stale entry cannot break create regardless of the current managed web-search choice. The existing null-config integration assertion covers the reported symptom but not this ownership contract.

The exact-head DCO gate failed in run 28988332864 because the PR body has only a checkbox. Please add the literal contributor declaration yourself: Signed-off-by: Ho Lim <subhoya@gmail.com>. The commit itself is already signed and GitHub-verified; this request is only for the required PR-body declaration.

Local clean-merge validation otherwise passed the focused planner (12/12) and onboard (65/65) suites. The manually dispatched advisors failed on provider 429s and produced no verdict; those infrastructure failures are not code blockers.

apurvvkumaria · 2026-07-09T17:13:34Z

Superseded by a clean current-main replacement that preserves this narrow implementation and credits Ho Lim as co-author. The replacement uses a GitHub-verified, signed/DCO-compliant commit and has completed targeted plus aggregate validation; I will link it here as soon as GitHub creates the PR.

apurvvkumaria · 2026-07-09T17:14:09Z

Replacement opened as #6587. It preserves the narrow provider-filtering implementation, credits Ho Lim as co-author, and uses a GitHub-verified signed/DCO-compliant commit from current main.

## Summary Reconcile registry-recorded extra providers against the authoritative OpenShell gateway list before sandbox creation. This prevents a stale `tavily-search` record from breaking onboarding while preserving every healthy gateway-wide user provider. The PR replaces #6531, preserves Ho Lim's original issue work, and credits Shawn Xie's provider-reconciliation direction from #6518. ## Related Issue Fixes #6501. ## Changes - Query `provider list -g <gateway> --names` once when registry extras exist. - Attach only exact provider names returned by a successful authoritative list. - Preserve all recorded providers if the gateway query fails or throws, avoiding silent user-state loss during an outage. - Keep local registry state unchanged; reconciliation affects only the current sandbox-create plan. - Preserve healthy bare `brave-search`, `tavily-search`, custom, and bridge providers without treating their names as NemoClaw-owned. - Add focused and spawn-level regressions for healthy providers, stale records, exact-name matching, gateway scoping, and fail-open behavior. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: this silently restores the documented provider source-of-truth contract without adding or changing a command, option, output, configuration, or remediation step. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — exact-name gateway reconciliation was independently audited; it avoids diagnostic regexes, preserves healthy user-owned providers, fails open on gateway-list failure, and does not mutate registry state. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior — 17 focused reconciliation tests and all 65 onboard integration tests passed. - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Additional exact-head validation: CLI type-check, file/commit/push hooks, commitlint, test-size/source-shape budgets, secret scan, and `npm run check:diff` passed. Documentation review found no update required. `src/lib/onboard.ts` is net-neutral. --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>  ## Summary by CodeRabbit * **New Features** * Sandbox creation now better matches configured providers to what’s actually available, reducing unexpected provider options during onboarding. * **Bug Fixes** * Fixed cases where stale or unavailable extra providers could still appear in sandbox setup. * Improved behavior when provider lookup fails, helping preserve previously configured choices instead of removing them unexpectedly.  --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Ho Lim <subhoya@gmail.com> Co-authored-by: Shawn Xie <shaxie@nvidia.com>

## Summary Reconcile registry-recorded extra providers against the authoritative OpenShell gateway list before sandbox creation. This prevents a stale `tavily-search` record from breaking onboarding while preserving every healthy gateway-wide user provider. The PR replaces NVIDIA#6531, preserves Ho Lim's original issue work, and credits Shawn Xie's provider-reconciliation direction from NVIDIA#6518. ## Related Issue Fixes NVIDIA#6501. ## Changes - Query `provider list -g <gateway> --names` once when registry extras exist. - Attach only exact provider names returned by a successful authoritative list. - Preserve all recorded providers if the gateway query fails or throws, avoiding silent user-state loss during an outage. - Keep local registry state unchanged; reconciliation affects only the current sandbox-create plan. - Preserve healthy bare `brave-search`, `tavily-search`, custom, and bridge providers without treating their names as NemoClaw-owned. - Add focused and spawn-level regressions for healthy providers, stale records, exact-name matching, gateway scoping, and fail-open behavior. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: this silently restores the documented provider source-of-truth contract without adding or changing a command, option, output, configuration, or remediation step. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — exact-name gateway reconciliation was independently audited; it avoids diagnostic regexes, preserves healthy user-owned providers, fails open on gateway-list failure, and does not mutate registry state. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior — 17 focused reconciliation tests and all 65 onboard integration tests passed. - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Additional exact-head validation: CLI type-check, file/commit/push hooks, commitlint, test-size/source-shape budgets, secret scan, and `npm run check:diff` passed. Documentation review found no update required. `src/lib/onboard.ts` is net-neutral. --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>  ## Summary by CodeRabbit * **New Features** * Sandbox creation now better matches configured providers to what’s actually available, reducing unexpected provider options during onboarding. * **Bug Fixes** * Fixed cases where stale or unavailable extra providers could still appear in sandbox setup. * Improved behavior when provider lookup fails, helping preserve previously configured choices instead of removing them unexpectedly.  --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Ho Lim <subhoya@gmail.com> Co-authored-by: Shawn Xie <shaxie@nvidia.com>

Copilot AI review requested due to automatic review settings July 9, 2026 01:36

Copilot AI reviewed Jul 9, 2026

HOYALIM force-pushed the codex/issue-6501-disable-web-search-provider branch from bb37e07 to e270203 Compare July 9, 2026 01:39

coderabbitai Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread src/lib/onboard/sandbox-create-plan.ts

fix(onboard): drop stale disabled web-search providers

81556b1

Signed-off-by: Ho Lim <subhoya@gmail.com>

HOYALIM force-pushed the codex/issue-6501-disable-web-search-provider branch from e270203 to 81556b1 Compare July 9, 2026 01:46

coderabbitai Bot reviewed Jul 9, 2026

View reviewed changes

This was referenced Jul 9, 2026

refactor(onboard): extract sandbox create-failure reporting and create step into modules #6444

Merged

fix(dcode): require Landlock enforcement #5812

Closed

apurvvkumaria added the v0.0.79 label Jul 9, 2026

apurvvkumaria self-assigned this Jul 9, 2026

cjagwani requested changes Jul 9, 2026

View reviewed changes

apurvvkumaria closed this Jul 9, 2026

apurvvkumaria mentioned this pull request Jul 9, 2026

fix(onboard): reconcile registered extra providers #6587

Merged

21 tasks

wscurran added area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow bug-fix PR fixes a bug or regression labels Jul 12, 2026

Navigation Menu

Uh oh!

fix(onboard): drop stale disabled web-search providers - #6531

fix(onboard): drop stale disabled web-search providers#6531
HOYALIM wants to merge 1 commit into
NVIDIA:mainfrom
HOYALIM:codex/issue-6501-disable-web-search-provider

HOYALIM commented Jul 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

copy-pr-bot Bot commented Jul 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

cjagwani left a comment

Uh oh!

apurvvkumaria commented Jul 9, 2026

Uh oh!

apurvvkumaria commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

HOYALIM commented Jul 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Tests

DCO

Summary by CodeRabbit

Uh oh!

copy-pr-bot Bot commented Jul 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cjagwani left a comment

Choose a reason for hiding this comment

Uh oh!

apurvvkumaria commented Jul 9, 2026

Uh oh!

apurvvkumaria commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

HOYALIM commented Jul 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading