ci(e2e): add exact-SHA Brev Launchable E2E lane#7043
Conversation
Signed-off-by: Charan Jagwani <cjagwani@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. |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 4b95fa4 in the TypeScript / code-coverage/cliThe overall coverage in commit 4b95fa4 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: 2 optional E2E recommendations
Blockers
|
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
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:
📝 WalkthroughWalkthroughAdds an exact-SHA staging qualification pipeline covering manifest validation, GitHub producer orchestration, Brev deployment and runtime smoke checks, cleanup, evidence publication, workflow integration, and extensive end-to-end tests. ChangesExact image qualification
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant E2EWorkflow
participant QualificationWorkflow
participant QualificationController
participant ProducerWorkflow
participant Brev
participant StagingRuntime
E2EWorkflow->>QualificationWorkflow: invoke with candidate SHA and reason
QualificationWorkflow->>QualificationController: preflight and start
QualificationController->>ProducerWorkflow: dispatch exact-image build
QualificationWorkflow->>QualificationController: wait, download, validate, finalize
QualificationWorkflow->>Brev: authenticate and bind staging workspace
QualificationWorkflow->>StagingRuntime: deploy and qualify Launchable
StagingRuntime-->>QualificationWorkflow: return runtime evidence
QualificationWorkflow->>Brev: clean up workspace
QualificationWorkflow-->>E2EWorkflow: publish qualification evidence
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
wscurran
added
area: ci
jyaunches
changed the title
…e-manifest-handoff
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
jyaunches
changed the title
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
test/brev-launchable-runtime.test.ts (1)
44-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExercise remote host commands instead of matching substrings.
The fake returns success whenever a command contains expected text, without executing its shell syntax. Broken quoting or malformed pipelines would still pass. Keep Brev and NVIDIA services mocked, but run
--hostcommands under Bash with fakesudo,curl,openshell, and related tools.As per path instructions, flag broad mocks that bypass the behavior under test.
Also applies to: 94-112
🤖 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/brev-launchable-runtime.test.ts` around lines 44 - 55, Replace the broad substring-based command handling in the exec mock with Bash execution for --host commands, using fake sudo, curl, openshell, and related service tools while retaining Brev and NVIDIA service responses. Ensure the mocked commands execute their actual shell syntax so quoting and pipeline errors surface, and update the corresponding setup also covered around lines 94-112.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 @.github/workflows/brev-launchable-qualification.yaml:
- Around line 225-239: Move the staging Launchable deploy and qualification
journey out of the standalone workflow steps and into a focused Vitest E2E test
that invokes the existing shell/runtime boundaries. Update the unified Vitest
workflow to select this test, and revise the source-shape contract plus its
allowed exception so the standalone workflow is no longer an additional E2E
path.
In @.github/workflows/e2e.yaml:
- Around line 192-196: Update the reusable workflow invocation calling
brev-launchable-qualification.yaml to replace secrets: inherit with an explicit
secrets mapping containing only NEMOCLAW_IMAGE_DISPATCH_TOKEN, BREV_API_KEY,
BREV_ORG_ID, and NVIDIA_INFERENCE_API_KEY, preserving each caller secret’s
value.
In `@tools/e2e/brev-launchable-runtime.sh`:
- Around line 113-125: Update the provision metadata validation near the jq
check and provision_sha extraction to require a non-empty producer-supported
abbreviated Git SHA format before accepting the metadata. Ensure the prefix
comparison only runs for a valid SHA and add a regression case covering empty
gitSha metadata, while preserving the existing candidate and full repository SHA
checks.
- Around line 82-84: Update the workspace existence check around
workspace_record so failures or malformed brev ls responses are treated as fatal
rather than as an empty inventory. Ensure the script only proceeds to brev
create after successfully validating that the requested workspace name is
unused, and call die with an appropriate error for inventory failures.
In `@tools/e2e/exact-image-qualification-controller.mts`:
- Around line 679-696: Update both persisted dispatch-intent validators to
accept any positive safe requester run attempt instead of requiring 1, and
remove the candidateSha/workflowSha equality check while retaining individual
SHA validation. Add regression coverage in the exact-image qualification
controller tests verifying start and cleanup succeed for both an attempt above 1
and differing candidate/workflow SHAs.
---
Nitpick comments:
In `@test/brev-launchable-runtime.test.ts`:
- Around line 44-55: Replace the broad substring-based command handling in the
exec mock with Bash execution for --host commands, using fake sudo, curl,
openshell, and related service tools while retaining Brev and NVIDIA service
responses. Ensure the mocked commands execute their actual shell syntax so
quoting and pipeline errors surface, and update the corresponding setup also
covered around lines 94-112.
🪄 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: d5278ccd-4a39-4fc6-a0ba-26214ba23307
📒 Files selected for processing (17)
.github/workflows/brev-launchable-qualification.yaml.github/workflows/e2e.yamlci/source-shape-test-budget.jsontest/brev-launchable-runtime.test.tstest/e2e/support/exact-image-manifest-cli.test.tstest/e2e/support/exact-image-manifest-fixture.tstest/e2e/support/exact-image-manifest.test.tstest/exact-image-qualification-controller.test.tstest/exact-image-qualification-workflow.test.tstest/helpers/exact-image-qualification-controller-fixture.tstest/helpers/vitest-watch-triggers.tstest/vitest-watch-triggers.test.tstools/advisors/github.mtstools/e2e/brev-launchable-runtime.shtools/e2e/exact-image-manifest.mtstools/e2e/exact-image-qualification-controller.mtstools/e2e/validate-exact-image-manifest.mts
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
The Phase 4 design pattern is now recorded as accepted in #6943: standing staging-family Launchable, post-boot exact image/SHA verification, preinstalled E2E, and cleanup, with the older per-candidate Launchable revision/browser/pre-provision/TTL requirements deferred for this slice. The implementation remains inactive by default and all exact-head code gates are complete. The remaining pre-merge action is formal human approval on this PR. Activation remains separate and must wait for the protected environments, WIF, standing Launchable configuration, operational ownership, and the required live |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
jyaunches
changed the title
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
Exact-head gate update for
I filed #7266 with the exact evidence and a narrowly bounded, fail-closed retry design for maintainer consideration. I have not rerun the same SHA, rebased solely to mint a new revision, or weakened the gate. Maintainer disposition requested: either accept #7266 as a separate fix to land before #7043 is refreshed and revalidated, or explicitly accept this exact non-success gate with #7266 as the follow-up. Formal PR approval remains separately required. The Launchable qualification feature is still inactive by default. |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
|
Closing as superseded by the narrowed, authoritative scope in #6943. #6943 now calls for one focused lane in the existing The actual Launchable coverage is still required: exact candidate -> existing #80 staging producer -> standing Launchable -> baked SHA and boot-image verification -> existing full E2E in preinstalled mode -> unconditional cleanup and minimal redacted evidence. That will be handled as a smaller direct E2E change rather than preserving this qualification subsystem. The Brev v0.6.330 interface findings and focused runtime tests in this branch remain available for the smaller replacement. #7266 remains a separate forwarding bug and is not part of that replacement. |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
…NVIDIA/NemoClaw into codex/pr-7043-simplify Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> # Conflicts: # test/brev-launchable-runtime.test.ts
…handoff' into codex/pr-7043-simplify Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
jyaunches
changed the title
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
|
Superseded by #7270, which implements the accepted one-focused-E2E-lane design within the agreed line budgets. Closing this larger draft to avoid parallel or generalized implementations. |
| timeout "${BREV_COPY_TIMEOUT_SECONDS:-300}" \ | ||
| brev copy "$INSTANCE_NAME:$remote_artifact_dir/" "$local_artifact_dir/" --host | ||
| local result="$local_artifact_dir/$result_relative" | ||
| [ -f "$result" ] && [ ! -L "$result" ] || die "full E2E result is missing" |
|
The accepted scope in #6943 has been reduced to exactly what this E2E lane needs. We do not need an image-qualification subsystem. The issue now requires one direct path in the existing
The standalone qualification workflow/controller, state machine, evidence ledger, generalized manifest protocol, and in-guest cloud image-ID lookup are explicit non-goals. The issue and this PR were moved to v0.0.91. The 6.6k-line implementation on this branch was reduced to 912 additions / 69 deletions across 10 files before this PR was closed as superseded by #7270. Live evidence from exact head
So the workflow code and fresh producer path are exercised, but the new E2E has not passed. The remaining blocker is external Launchable configuration: it must actually boot the staging-family image before the existing full E2E can start. #7270 currently retains the same unavailable service-account image lookup and 10-minute cleanup deadline, so this live evidence should be applied there if #7270 remains the replacement. |
6 participants
Summary
Adds one explicit Brev Launchable lane to the existing E2E workflow. The lane builds or reuses the staging CPU image for the exact candidate SHA, proves what the Launchable actually booted, runs the existing full E2E suite from the baked checkout, and deletes the workspace.
This revision removes the standalone qualification workflow, controller, state machine, evidence ledger, and generalized image-manifest protocol. Those layers are not needed for this workflow.
Related Issue
Fixes #6943.
Changes
staging-brev-launchablejob directly to.github/workflows/e2e.yaml.brevdev/nemoclaw-imagestaging build for the exact candidate SHA and waits for that exact producer run.test/e2e/live/full-e2e.test.tssuite inpreinstalled-launchablemode.install.sh, install dependencies, rebuild, relink, promote an image, or repoint a Launchable.Target release:
v0.0.91.Type of Change
Quality Gates
test/e2e/README.mdVerification
Signed-off-by:line; GitHub commit verification is checked after each pushpre-commit,commit-msg, andpre-pushhooks passednpm run test:changed; CLI typecheck; Vitest project membership; source-shape; ShellCheck; shfmt; Biome;git diff --checknpm run docsbuilds without warnings (doc changes only)Live E2E status
Run 29785815949 built the fresh exact-SHA image successfully, deployed a READY/HEALTHY workspace, and confirmed cleanup after failure. It did not reach Vitest: the VM had no service-account token for the attempted cloud image lookup, and a bounded follow-up deploy confirmed the configured Launchable currently supplies neither
/etc/nemoclaw/provision.jsonnor$HOME/NemoClaw.This PR is closed as superseded by #7270. The remaining external dependency is a standing Launchable that actually boots the staging-family image; the live evidence and simplified v0.0.91 acceptance criteria are recorded in the closing comment.
Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Signed-off-by: Julie Yaunches jyaunches@nvidia.com