test(e2e): clarify Brev bootstrap boundaries - #7110
Conversation
|
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. |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe E2E workflows and tests now use Brev bootstrap installation and source validation instead of published Launchable paths. Launchable inputs, environment wiring, job names, artifacts, readiness checks, and related validation references were replaced with bootstrap-specific equivalents. ChangesBrev bootstrap E2E validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow as e2e-branch-validation
participant Brev as Generic Brev VM
participant Bootstrap as Brev bootstrap script
participant Smoke as bootstrap-install-smoke
Workflow->>Brev: provision generic instance
Brev->>Bootstrap: execute bootstrap installation
Bootstrap-->>Smoke: create BOOTSTRAP_SENTINEL
Smoke->>Brev: validate installation and source state
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit c0aed78 in the TypeScript / code-coverage/cliThe overall coverage in commit c0aed78 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — Blocking findings reportedAdvisor assessment: Blockers require maintainer review Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: Blockers
|
wscurran
added
area: ci
|
Deferred from v0.0.88 while this remains author-marked draft. The missing live-E2E parity registry entry has now been added directly, the branch is refreshed, 187 focused tests plus project-membership and the full diff gate pass, and fresh CI is running. No substantive code blocker remains from this pass; return it to review after the author marks it ready and required checks are green. |
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Reviewed exact head d29875ad against current main (5b547cdf). One correctness blocker remains.
The trusted E2E controller on main intentionally includes a renamed file's previous_filename, so the current coordination plan selects launchable-smoke. The dispatched trusted-main workflow then checks out this PR head and invokes test/e2e/live/launchable-smoke.test.ts, but this head deletes that path and contains only test/e2e/live/bootstrap-install-smoke.test.ts. Authorizing the selected exact-head job would therefore fail predictably with a missing test path.
Please stage the rename compatibly so the trusted-main job inventory remains executable (or add equivalent controller-safe handling with regression coverage), and merge current main into the branch without rewriting its verified history. The refresh appears mechanically feasible, but it does not by itself resolve the old-job/new-path incompatibility. Fresh CI, CodeRabbit, and exact E2E should then run before approval.
The rest of the code/security review is clean, and this internal terminology correction does not create a new supported product surface.
|
Update: the requested compatibility structure is now correct at exact head The required repository E2E authorization was generated for base |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e/live/bootstrap-install-smoke.test.ts (1)
4-8: 📐 Maintainability & Code Quality | 🔵 TrivialTrack retirement of the legacy
launchable-smoke.test.tspath.This alias plus the still-fully-functional
launchable-smoke.test.tsis an intentional staging step per the PR description, but per path instructions fortest/e2e/**, a migration should eventually prove the superseded path is unreachable/removed rather than leaving both live indefinitely. Worth tracking a follow-up to deletelaunchable-smoke.test.ts's implementation once the trusted main-branch workflow no longer dispatches by the old filename.As per path instructions, "Migration tests must prove the superseded path is unreachable or removed, not merely prove that the new path also works."
🤖 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 `@test/e2e/live/bootstrap-install-smoke.test.ts` around lines 4 - 8, Track the migration in the target-specific bootstrap-install-smoke path: once the trusted main-branch workflow stops dispatching the legacy filename, remove the launchable-smoke.test.ts implementation and this compatibility alias, ensuring the superseded path is unreachable rather than retaining both test entry points.Source: Path instructions
🤖 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 `@test/e2e/live/launchable-smoke.test.ts`:
- Around line 24-28: Fix the design-rationale comment above the live test by
joining the trailing “through Vitest” fragment to the preceding sentence, so it
reads as one grammatically complete statement describing the OpenClaw agent turn
being run through Vitest.
---
Nitpick comments:
In `@test/e2e/live/bootstrap-install-smoke.test.ts`:
- Around line 4-8: Track the migration in the target-specific
bootstrap-install-smoke path: once the trusted main-branch workflow stops
dispatching the legacy filename, remove the launchable-smoke.test.ts
implementation and this compatibility alias, ensuring the superseded path is
unreachable rather than retaining both test entry points.
🪄 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: 9bbc9c56-4d12-4eba-89b9-a5ea560e9dc9
📒 Files selected for processing (3)
test/e2e/live/bootstrap-install-smoke.test.tstest/e2e/live/launchable-smoke.test.tstest/e2e/support/e2e-live-target-gating.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/support/e2e-live-target-gating.test.ts
|
PR Review Advisor
There is no runtime state where the old trusted inventory dispatches the renamed head workflow. Keeping a permanent |
|
That is why the bridge belongs in the checked-out planner: before merge, the trusted graph selects The prior downstream run 29764931487 independently confirms the workflow-revision boundary: its parsed job graph contains |
later comment acknowledge requested changes came in
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical `## v0.0.90` entry to `docs/changelog/2026-07-20.mdx` before the release tag is planned. The update also corrects the documented custom-image migration window so the compatibility fallback that first ships in v0.0.90 remains available until v0.0.91. ## Changes - Add the v0.0.90 summary and detailed release bullets for managed-image routing, provider-reset recovery, WhatsApp health reporting, and DGX Station guidance. - Keep the newest release first in the shared dated changelog and use root-absolute links to the canonical OpenClaw routes. - Correct `docs/reference/commands.mdx` to state that the legacy image route selector remains supported through v0.0.90 and is removed in v0.0.91. - Release source summary: - [#7264](#7264) -> `docs/resources/prompt-assets/dgx-station.md`, `docs/changelog/2026-07-20.mdx`: Record the versioned Station installer path, Nemotron 3 Ultra 550B default, and explicit DeepSeek override. - [#7261](#7261) -> `docs/get-started/dgx-station-preparation.mdx`, `docs/manage-sandboxes/recover-rebuild-sandboxes.mdx`, `docs/changelog/2026-07-20.mdx`: Include the OpenIB, legacy recovery, and Additional Setup documentation follow-ups. - [#7232](#7232) -> `docs/changelog/2026-07-20.mdx`: Document provider-reset recovery for wrapped OpenShell attachment diagnostics. - [#7189](#7189) -> `docs/reference/commands.mdx`, `docs/changelog/2026-07-20.mdx`: Document the managed-image route-selector rename and correct its one-release migration window. - [#7015](#7015) -> `docs/changelog/2026-07-20.mdx`: Document corrected OpenClaw WhatsApp health reporting. - No additional user-facing page update is needed for [#7193](#7193), [#7110](#7110), [#6783](#6783), or [#7263](#7263) because they change contributor governance, internal CI or release automation, or editorial style without changing supported user behavior. - [#7242](#7242) and [#7225](#7225) are already ancestors of and documented in v0.0.89, so this entry does not duplicate them despite their stale v0.0.90 labels. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog heading, SPDX form, version order, and published links. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; this PR does not change `scripts/prepare-dgx-station-host.sh` or runtime behavior. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts` (6 passed). - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not run; this is a focused documentation-only change. - [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) — passed with 0 errors and two unrelated baseline warnings for unauthenticated redirect checks and the existing light-mode contrast ratio. - [x] 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) --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Added release notes for v0.0.90 covering inference routing, credential reset behavior, WhatsApp status detection, and DGX Station coding-agent guidance. - Updated custom Dockerfile guidance to document continued support for the legacy provider argument through v0.0.90. - Clarified that legacy declarations must be renamed to `NEMOCLAW_INFERENCE_PROVIDER_ID` before v0.0.91. - Added and refreshed related documentation links. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
5 participants
Summary
Clarifies which E2E paths exercise a published Brev Launchable and which paths provision a generic Brev instance, bootstrap NemoClaw from source, and test that source overlay. This removes misleading Launchable terminology without changing image selection, deployment, authentication, or test behavior.
Related Issue
Related to #6943
Changes
launchable-smoketobootstrap-install-smoke.use_launchableinput from the reusable branch-validation workflow and its nightly/regression callers.scripts/brev-launchable-ci-cpu.shunchanged because its exact path and content are part of the installer trust boundary.Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project integration test/brev-launchable-ci-cpu-checksum.test.ts test/brev-nightly-workflow.test.ts test/dependency-pins-check.test.ts test/installer-hash-check.test.ts test/openshell-channel-workflow.test.ts test/pr-workflow-contract.test.ts test/runner.test.ts(184 passed after merging current main);npx vitest run --project e2e-support test/e2e/support/prepare-e2e-workflow-boundary.test.ts(4 passed);npx vitest run --project e2e-support test/e2e/support/e2e-live-target-gating.test.ts(5 passed);npx vitest run --project integration test/e2e-mock-parity.test.ts test/pr-e2e-gate.test.ts(36 passed);npm run test:projects:checkpassed (1698 candidate files across 8 projects). Current selector-bridge regression: npx vitest run --project e2e-support test/e2e/support/workflow-plan.test.ts (20 passed); adjacent workflow/planner consumers (70 passed).npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — attempted; the local run encountered unrelated widespread timeouts and missing oclif development plugins, so this remains unchecked pending GitHub CI.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Julie Yaunches jyaunches@nvidia.com
Summary by CodeRabbit
New Features
bootstrap-install-smoke), including updated PR reporting readiness.Bug Fixes
use_launchablefrom E2E workflow inputs and updated readiness/sentinel handling to match the bootstrap path.Tests
bootstrap-install-smoke.