Skip to content

test(e2e): migrate Brave search to vitest - #5552

Merged
cv merged 12 commits into
mainfrom
e2e-migrate/test-brave-search
Jun 20, 2026
Merged

test(e2e): migrate Brave search to vitest#5552
cv merged 12 commits into
mainfrom
e2e-migrate/test-brave-search

Conversation

@cv

@cv cv commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Migrates test/e2e/test-brave-search-e2e.sh into a typed live Vitest scenario. The new coverage preserves the #2687 Brave Search acceptance path: onboard with a real BRAVE_API_KEY, validate policy/config wiring, assert the real key is not exposed in sandbox state/env, and exercise both an OpenClaw agent search and a direct in-sandbox Brave API curl.

Related Issue

Refs #5098

Changes

  • Add test/e2e-scenario/live/brave-search.test.ts with typed cleanup, artifacts, and secret redaction.
  • Wire brave-search-vitest into .github/workflows/e2e-vitest-scenarios.yaml as a free-standing dispatchable Vitest job.
  • Preserve legacy shell deletion and nightly shell wiring for Phase 11 cleanup per Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 migration governance.

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

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run 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)

Targeted commands run:

  • npx biome check --write test/e2e-scenario/live/brave-search.test.ts
  • NEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/brave-search.test.ts -t __compile_only_nomatch__ --silent=false --reporter=default --passWithNoTests
  • npx tsc --noEmit --strict --moduleResolution bundler --module preserve --target ES2022 --types node --allowImportingTsExtensions test/e2e-scenario/live/brave-search.test.ts
  • npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts
  • npx tsx scripts/check-test-file-size-budget.ts test/e2e-scenario/live/brave-search.test.ts
  • git diff --check

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

Summary by CodeRabbit

  • Tests

    • Added a live end-to-end Brave Search Vitest scenario using real API credentials for onboarding, with retry/backoff for transient validation issues.
    • Verifies sandbox policy and configuration, safe redaction of secrets via placeholders, correct key placeholder matching, and successful Brave-backed web search both in-sandbox and via direct endpoint calls.
  • CI / Workflow

    • Added a dedicated Brave Search free-standing Vitest live job, using the same scenario selection gating, and now included its results in the pull request status table.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this Jun 19, 2026
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds test/e2e-scenario/live/brave-search.test.ts, a Vitest live E2E test that provisions an OpenShell sandbox, validates Brave Search policy and config correctness, asserts key redaction, and verifies both agent-driven and direct curl Brave API calls. A new brave-search-vitest CI job is wired into .github/workflows/e2e-vitest-scenarios.yaml with the existing selector gating, and report-to-pr is updated to include the new job in its needs list.

Changes

Brave Search E2E Migration

Layer / File(s) Summary
Test infrastructure and helpers
test/e2e-scenario/live/brave-search.test.ts
Defines module-level constants (commandEnv, placeholder regexes, timeout), and helper functions: bestEffort (cleanup error suppression), singleLineShell (base64-encoded one-shot payloads), sandboxShell (run scripts in sandbox with timeout/redaction), cleanupBraveSandbox (OpenShell destroy), parsePlaceholder (extract key from config), and extractOpenClawAgentText (tolerant JSON parsing for agent output).
Live E2E test body and assertions
test/e2e-scenario/live/brave-search.test.ts
Implements the skipped-by-default Vitest flow: Docker validation with CI-aware skip/throw, onboarding with retry/backoff, sandbox policy verification for api.search.brave.com, config assertions (provider, placeholder pattern, key redaction), in-sandbox OpenClaw agent web search with NVIDIA/GPU text validation, and direct curl to Brave API with placeholder token header asserting HTTP 200 and non-empty results.
CI job wiring
.github/workflows/e2e-vitest-scenarios.yaml
Adds the brave-search-vitest job with inputs.jobs/inputs.scenarios gating, OpenShell installation and binary discovery, Vitest execution of brave-search.test.ts with BRAVE_API_KEY/NVIDIA_INFERENCE_API_KEY secrets, artifact upload to e2e-artifacts/vitest/brave-search/, and inclusion in report-to-pr.needs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

  • NVIDIA/NemoClaw#5243: Introduced the shared Vitest job-selector plumbing in e2e-vitest-scenarios.yaml that this PR's brave-search-vitest job depends on.
  • NVIDIA/NemoClaw#5346: Adds snapshot-commands-vitest to the same workflow following the same free-standing job and report-to-pr.needs pattern.
  • NVIDIA/NemoClaw#5493: Adds full-e2e-vitest to the same workflow using the same CI wiring pattern and report-to-pr.needs extension.

Suggested labels

area: e2e, chore

🐇 A brave little rabbit searched the web today,
With API keys hidden safely away.
The sandbox was built, the curl returned 200,
The titles said NVIDIA — results were plenty!
No real key exposed, just a placeholder token,
The legacy script's promise: cleanly unbroken. 🔍✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'test(e2e): migrate Brave search to vitest' directly summarizes the main change: migrating an end-to-end test from a shell script to Vitest/TypeScript. It is concise, specific, and clearly conveys the primary objective.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch e2e-migrate/test-brave-search

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

@github-code-quality

github-code-quality Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the e2e-migrate/test-bra... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main e2e-migrate/test-bra... eda4974 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the e2e-migrate/test-bra... branch is 46%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main e2e-migrate/test-bra... eda4974 +/-
src/lib/state/o...oard-session.ts 90%
src/lib/inference/local.ts 76%
src/lib/sandbox/config.ts 72%
src/lib/actions...dbox/rebuild.ts 67%
src/lib/onboard/preflight.ts 64%
src/lib/actions...licy-channel.ts 56%
src/lib/state/sandbox.ts 55%
src/lib/policy/index.ts 49%
src/lib/onboard...er-gpu-patch.ts 44%
src/lib/onboard.ts 18%

Updated June 20, 2026 17:34 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: brave-search-vitest

Dispatch hint: brave-search-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None. Only E2E workflow/test files changed, so no merge-blocking runtime E2E is required. The new brave-search-vitest job is recommended as an optional validation of the added coverage and CI wiring.

Optional E2E

  • brave-search-vitest (live external-provider test; requires BRAVE_API_KEY and NVIDIA_INFERENCE_API_KEY secrets): Run the newly added E2E job to validate the workflow wiring and the new live Brave search scenario, including onboarding, policy/config assertions, credential redaction/leak checks, sandbox OpenClaw search, and direct Brave API access.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: .github/workflows/e2e-vitest-scenarios.yaml
  • jobs input: brave-search-vitest

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: brave-search-vitest
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=brave-search-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • brave-search-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/brave-search.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=brave-search-vitest

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • .github/workflows/e2e-vitest-scenarios.yaml
  • test/e2e-scenario/live/brave-search-helpers.ts
  • test/e2e-scenario/live/brave-search.test.ts

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 1 needs attention, 1 worth checking, 0 nice ideas
Since last review: 0 prior items resolved, 2 still apply, 0 new items found

Review findings

🛠️ Needs attention

  • B4a agent search assertion can miss the final assistant response (test/e2e-scenario/live/brave-search-helpers.ts:144): The migrated B4a clause runs `openclaw agent --json`, but `extractOpenClawAgentText()` still collects text from only `firstJsonObject(output)`. The retained shell helper scans JSON candidates until assistant text is found, which matters because launcher/progress JSON can appear before the final assistant envelope. In that case the new test can fail despite a valid Brave result, or assert on metadata rather than the assistant response.
    • Recommendation: Mirror `test/e2e/lib/openclaw-json.sh::parse_openclaw_agent_text`: scan parseable JSON objects until assistant/result text is found, avoid satisfying assertions from progress metadata, and add focused non-live regression coverage for progress JSON before the final response.
    • Evidence: `extractOpenClawAgentText(output)` returns `collectAssistantText(firstJsonObject(output))[0] ?? ""`; legacy `parse_openclaw_agent_text()` uses `json.JSONDecoder().raw_decode` over JSON candidates and breaks only after collected assistant text increases.

🔎 Worth checking

  • Source-of-truth review needed for tolerant OpenClaw agent JSON extraction (test/e2e-scenario/live/brave-search-helpers.ts:93): The localized tolerant parser handles wrapper/non-final JSON output, but the change does not establish why this logic belongs in the Brave helper instead of reusing or porting the retained shell parser, what invalid output state is expected, or what regression proves the behavior cannot drift again.
    • Recommendation: Either make the invalid state impossible at the source, or port/reuse the retained parser semantics with a small regression test and document the source boundary and removal condition for the workaround.
    • Evidence: `firstJsonObject()` catches parse failures and returns `undefined`; `extractOpenClawAgentText()` consumes only that first object, while the retained source helper scans raw JSON candidates until assistant text is collected.

🌱 Nice ideas

  • None.
Consider writing more tests for
  • **Runtime validation** — extractOpenClawAgentText skips a progress JSON object without assistant text and returns the later final assistant text. The PR changes a secret-bearing GitHub Actions workflow and a live sandbox/provider scenario. The live path covers the main Brave behavior, but the localized JSON extraction needs focused non-live coverage so the live B4a assertion is neither flaky nor satisfied by metadata.
  • **Runtime validation** — extractOpenClawAgentText does not satisfy Brave-search assertions from launcher/progress metadata strings alone. The PR changes a secret-bearing GitHub Actions workflow and a live sandbox/provider scenario. The live path covers the main Brave behavior, but the localized JSON extraction needs focused non-live coverage so the live B4a assertion is neither flaky nor satisfied by metadata.
  • **Runtime validation** — workflow dispatch scenarios=brave-search selects brave-search-vitest and leaves the registry matrix empty. The PR changes a secret-bearing GitHub Actions workflow and a live sandbox/provider scenario. The live path covers the main Brave behavior, but the localized JSON extraction needs focused non-live coverage so the live B4a assertion is neither flaky nor satisfied by metadata.
  • **Runtime validation** — assertBraveConfig rejects a non-placeholder apiKey value before the direct Brave curl command is built. The PR changes a secret-bearing GitHub Actions workflow and a live sandbox/provider scenario. The live path covers the main Brave behavior, but the localized JSON extraction needs focused non-live coverage so the live B4a assertion is neither flaky nor satisfied by metadata.
  • **Acceptance clause:** B4a Real Brave search via openclaw agent — add test evidence or identify existing coverage. `brave-search.test.ts` runs `openclaw agent --agent main --json` and asserts NVIDIA-related text, but the migrated parser only reads the first JSON object and can miss the final assistant envelope when progress JSON appears first.
  • **Tolerant OpenClaw agent JSON extraction** — Missing focused non-live regression showing progress JSON without assistant text before final response JSON returns the final assistant text.. `extractOpenClawAgentText()` consumes `firstJsonObject(output)` only; the retained shell parser scans raw JSON candidates until assistant text is collected.
Since last review details

Current findings:

  • B4a agent search assertion can miss the final assistant response (test/e2e-scenario/live/brave-search-helpers.ts:144): The migrated B4a clause runs `openclaw agent --json`, but `extractOpenClawAgentText()` still collects text from only `firstJsonObject(output)`. The retained shell helper scans JSON candidates until assistant text is found, which matters because launcher/progress JSON can appear before the final assistant envelope. In that case the new test can fail despite a valid Brave result, or assert on metadata rather than the assistant response.
    • Recommendation: Mirror `test/e2e/lib/openclaw-json.sh::parse_openclaw_agent_text`: scan parseable JSON objects until assistant/result text is found, avoid satisfying assertions from progress metadata, and add focused non-live regression coverage for progress JSON before the final response.
    • Evidence: `extractOpenClawAgentText(output)` returns `collectAssistantText(firstJsonObject(output))[0] ?? ""`; legacy `parse_openclaw_agent_text()` uses `json.JSONDecoder().raw_decode` over JSON candidates and breaks only after collected assistant text increases.
  • Source-of-truth review needed for tolerant OpenClaw agent JSON extraction (test/e2e-scenario/live/brave-search-helpers.ts:93): The localized tolerant parser handles wrapper/non-final JSON output, but the change does not establish why this logic belongs in the Brave helper instead of reusing or porting the retained shell parser, what invalid output state is expected, or what regression proves the behavior cannot drift again.
    • Recommendation: Either make the invalid state impossible at the source, or port/reuse the retained parser semantics with a small regression test and document the source boundary and removal condition for the workaround.
    • Evidence: `firstJsonObject()` catches parse failures and returns `undefined`; `extractOpenClawAgentText()` consumes only that first object, while the retained source helper scans raw JSON candidates until assistant text is collected.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv added the v0.0.66 label Jun 19, 2026
@cv cv linked an issue Jun 19, 2026 that may be closed by this pull request
79 tasks
cv added 3 commits June 19, 2026 14:07
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
…e-search

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv added 7 commits June 19, 2026 14:44
…e-search

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
…nto e2e-migrate/test-brave-search

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv merged commit b3a987a into main Jun 20, 2026
40 checks passed
@cv
cv deleted the e2e-migrate/test-brave-search branch June 20, 2026 18:43
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure integration: brave Brave integration behavior labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure integration: brave Brave integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants