Skip to content

refactor(onboard): extract web search configuration flow#3304

Merged
cjagwani merged 7 commits into
refactor/onboard-web-search-supportfrom
refactor/onboard-web-search-config
May 13, 2026
Merged

refactor(onboard): extract web search configuration flow#3304
cjagwani merged 7 commits into
refactor/onboard-web-search-supportfrom
refactor/onboard-web-search-config

Conversation

@cv
Copy link
Copy Markdown
Collaborator

@cv cv commented May 9, 2026

Summary

Extract the Brave web-search configuration flow out of the large onboarding module. This continues the onboarding cleanup stack by moving validation, prompting, credential persistence, and configuration selection into a focused helper module with direct coverage.

Changes

  • Add src/lib/onboard/web-search-config.ts for Brave Search validation, recovery prompting, credential setup, and web-search config selection.
  • Update src/lib/onboard.ts to use the extracted helper factory while preserving existing exports and call sites.
  • Add unit tests covering Brave validation curl arguments, unsupported-agent skip behavior, existing config reuse, non-interactive env setup, interactive credential prompting, and skip recovery.

Type of Change

  • 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)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • make docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

@cv cv self-assigned this May 9, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 9, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 01ad4d0c-7d3c-4b1d-ab8c-139472f511e5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/onboard-web-search-config

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

@cv cv marked this pull request as draft May 9, 2026 03:24
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 9, 2026

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.

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 9, 2026

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.

@cv cv added the v0.0.39 Release target label May 9, 2026
@wscurran wscurran added refactor This is a refactor of the code and/or architecture. and removed v0.0.39 Release target labels May 11, 2026
@cv cv added the v0.0.39 Release target label May 11, 2026
Comment thread src/lib/onboard.ts
} = validation;

const {
validateBraveSearchApiKey,
@cv cv added v0.0.40 Release target and removed v0.0.39 Release target labels May 12, 2026
Copy link
Copy Markdown
Contributor

@cjagwani cjagwani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm — clean dep-injection extraction. five funcs moved 1:1, both call sites at onboard.ts:10898 and :10904 match the new signatures, and the env capture via env = deps.env ?? process.env keeps the live mutation semantics intact.

two nits, neither blocking:

  • codeql is right at src/lib/onboard.ts:1874 — validateBraveSearchApiKey is destructured but never used locally and not re-exported. dead destructure, drop it (or add to module.exports if you want external consumers later).
  • web-search-config.ts:188 swapped isAffirmativeAnswer(enableAnswer) for an inline ["y","yes"].includes(...). prompt() always returns a string today so no live bug, but you lost the String(value||"") null guard. either keep the wrap or thread isAffirmativeAnswer through deps for parity with the rest of onboard.

test gap worth a follow-up: nothing covers the validation→retry→success path or the non-interactive throw. happy paths are good though.

stack base on #3302 looks right, ship when #3302 lands.

@cjagwani cjagwani marked this pull request as ready for review May 13, 2026 02:42
@cjagwani cjagwani merged commit 8bb073b into refactor/onboard-web-search-support May 13, 2026
19 checks passed
@cv cv deleted the refactor/onboard-web-search-config branch May 27, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor This is a refactor of the code and/or architecture. v0.0.40 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants