ci(e2e): include shared CLI runtime in artifact - #8337
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
📝 WalkthroughWalkthroughThe E2E CLI artifact now packages and restores ChangesCLI artifact shared runtime
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant E2EWorkflow
participant RestoreAction
participant Workspace
participant VersionCheck
E2EWorkflow->>RestoreAction: provide CLI artifact with dist and nemoclaw/dist/shared
RestoreAction->>RestoreAction: validate shared runtime modules and target paths
RestoreAction->>Workspace: restore dist and nemoclaw/dist
Workspace->>VersionCheck: expose restored CLI
VersionCheck->>Workspace: verify CLI version
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 2⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Closing as superseded by #8335, which merged the complete CLI build artifact restore fix into main before this PR opened. Post-merge E2E run 31008970541 confirms the artifact path is fixed: generate-matrix packaged successfully and consumer jobs restored the exact-commit CLI artifact successfully. The remaining run failure is unrelated (Deep Agents Code status assertion). |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
3 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
3 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/support/cli-artifact-workflow-boundary.test.ts (1)
88-129: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover each required shared module in the failure and success cases.
writeCliArchiveonly removes the complete shared runtime. The success assertion checks onlysandbox-name.cjs. A regression that stops requiringopenshell-policy-boundary.cjsorsnapshot-sanitizer-boundary.cjscan pass this suite. Add a fixture that omits each module separately and assert all three modules after a successful restore.As per path instructions, review tests for behavioral confidence rather than implementation lock-in.
Also applies to: 437-478
🤖 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/support/cli-artifact-workflow-boundary.test.ts` around lines 88 - 129, Expand the archive fixtures around writeCliArchive to support omitting each shared module independently, then add failure cases proving restore rejects archives missing openshell-policy-boundary.cjs, sandbox-name.cjs, or snapshot-sanitizer-boundary.cjs. Update the successful restore assertion to verify all three modules are required and available, while keeping the checks focused on observable behavior rather than implementation details.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.
Nitpick comments:
In `@test/e2e/support/cli-artifact-workflow-boundary.test.ts`:
- Around line 88-129: Expand the archive fixtures around writeCliArchive to
support omitting each shared module independently, then add failure cases
proving restore rejects archives missing openshell-policy-boundary.cjs,
sandbox-name.cjs, or snapshot-sanitizer-boundary.cjs. Update the successful
restore assertion to verify all three modules are required and available, while
keeping the checks focused on observable behavior rather than implementation
details.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 797aacbf-88a3-45b4-b635-6d3e827a376a
📒 Files selected for processing (5)
.github/actions/restore-e2e-cli-artifact/action.yaml.github/workflows/e2e.yamltest/e2e/README.mdtest/e2e/support/cli-artifact-workflow-boundary.test.tstools/e2e/cli-artifact-workflow-boundary.mts
1 participant
Summary
The shared E2E artifact restored
dist/but omitted the generated modules undernemoclaw/dist/shared/, so artifact consumers stopped before tests withMODULE_NOT_FOUND. The artifact now packages, validates, and restores both compiled roots before the CLI version check.Changes
dist/andnemoclaw/dist/shared/in the content-addressed candidate CLI artifact.ba10383ce965a78b470e15c65948bbf739bfb0ca.test/e2e/README.mdwith the two-root artifact and restore contract.Type of Change
Quality Gates
test/e2e/README.mddocuments that contract; no user-facing command, configuration, or output changes.Documentation Writer Review
docs-updatedtest/e2e/README.mddocuments the two-root candidate CLI artifact, validation requirements, and restore behavior.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run --project e2e-support test/e2e/support/cli-artifact-workflow-boundary.test.ts test/e2e/support/prepare-e2e-workflow-boundary.test.ts test/e2e/support/runner-comparison-workflow-boundary.test.ts— 3 files, 69 tests passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: San Dang sdang@nvidia.com
Summary by CodeRabbit