Skip to content

fix(ci): retain locked dependencies during E2E SDK installation - #11451

Closed
ericksoa wants to merge 1 commit into
mainfrom
fix/11449-e2e-sdk-lockfile
Closed

fix(ci): retain locked dependencies during E2E SDK installation#11451
ericksoa wants to merge 1 commit into
mainfrom
fix/11449-e2e-sdk-lockfile

Conversation

@ericksoa

@ericksoa ericksoa commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Outcome

E2E jobs install the reviewed OpenShell SDK archive using the dependency graph already installed by npm ci. This removes the shared edgesOut setup failure that prevented the runtime tests from starting.

Reason

--package-lock=false disables lockfile reading as well as writing. During the additional SDK install, npm 10.9.8 resolves the dependency graph again and crashes in Arborist. The existing mocked-npm test checked the command and credential removal but could not detect dependency-resolution failures.

Related issues

Fixes #11449. Unblocks qualification of #11382 and #11366.

Changes

  • Remove --package-lock=false from the catalogue profile and external gateway health job. Retain --no-save, --ignore-scripts, archive cardinality checks, and credential removal.
  • Update the catalogue validator's expected command without relaxing its other requirements.
  • Exercise both workflow scripts with real npm, an isolated offline cache, a locked dependency, and an SDK archive. Assert successful installation and unchanged manifests. The fixture's install script fails if lifecycle execution is enabled.
  • Select these regressions when either owning workflow changes.

Verification

  • Real Linux ARM64 container, Node 22.23.2 / npm 10.9.8, main 7b4d000499e3abbedf51d817a8771588e69ecafc package files, and SDK 0.0.106 from run 34487048057. The archive matched ci/reviewed-npm-audit.json SHA-512 integrity. No host credentials were mounted or passed. Fresh npm ci --ignore-scripts succeeded in both cases. The old command failed with edgesOut; the correction installed the SDK and passed the connection API import in 1.92 seconds. Both manifests remained byte-identical.
  • SDK regressions: two failures before the fix; all six tests pass after it. All 16 standard-profile boundary tests also pass.
  • Watch-trigger tests: 77 pass. Operations workflow tests pass. A wider local run hit two unchanged shared-workflow test deadlines during concurrent build work; all 11 shared-workflow tests pass in isolation. The original timeout results remain recorded.
  • CLI and plugin builds pass. CLI type-check passes with an 8 GiB Node heap; the default heap exhausted memory.
  • npm run validate:pr passes with canonical-main validation code and resolved executables. The changed boundary definition and watch mapping were temporarily replaced by canonical bytes for this independent gate, then restored. Their proposed behavior was tested separately above.
  • The diff contains no secrets, API keys, or credentials.

Review notes

Self-review covered all six changed files at 18b9f1f1f7c45c27bff72024ffa2d03aee0511d8 in NVIDIA/NemoClaw. The two workflows and tools/e2e/standard-profile-workflow-boundary.mts are sensitive paths. Independent review is pending; this PR remains a draft.

This repairs shared test setup. It does not establish live qualification for #11382: its 14 selected Docker/Podman jobs must run again after the repair reaches the trusted main workflow. Advisor review is also blocked by shared provider-budget exhaustion; no waiver is claimed.


Signed-off-by: Aaron Erickson aerickson@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Improved SDK archive installation to correctly handle package-lock files while continuing to avoid saving dependencies or running lifecycle scripts.
  • Tests

    • Added offline end-to-end coverage for SDK installation, dependency versions, and manifest integrity.
    • Updated workflow checks to ensure SDK installation scenarios run consistently across supported environments.

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa ericksoa self-assigned this Sep 10, 2026
@copy-pr-bot

copy-pr-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

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.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b7801c6d-b73f-4f63-bd0f-1b146a638844

📥 Commits

Reviewing files that changed from the base of the PR and between 3e6fabc and 18b9f1f.

📒 Files selected for processing (6)
  • .github/workflows/e2e-standard-profile.yaml
  • .github/workflows/e2e.yaml
  • test/e2e/support/openshell-sdk-install.test.ts
  • test/helpers/vitest-watch-triggers.ts
  • test/repository/vitest-watch-triggers.test.ts
  • tools/e2e/standard-profile-workflow-boundary.mts

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The E2E SDK installation commands now preserve the existing npm lockfile. Tests validate both workflow commands with offline real-npm installation, and workflow trigger contracts run the new SDK installation regression test.

Changes

SDK installation correction

Layer / File(s) Summary
SDK installation command correction
.github/workflows/e2e-standard-profile.yaml, .github/workflows/e2e.yaml, tools/e2e/standard-profile-workflow-boundary.mts
Both workflows and the boundary validator remove --package-lock=false while retaining --no-save and --ignore-scripts.
Offline SDK installation regression coverage
test/e2e/support/openshell-sdk-install.test.ts
The test extracts both workflow commands and verifies offline locked dependency installation, archive installation, unchanged manifests, and installed package versions.
Workflow contract trigger wiring
test/helpers/vitest-watch-triggers.ts, test/repository/vitest-watch-triggers.test.ts
The standard-profile workflow trigger and its expected contract inputs include the SDK installation test.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Severity of issue fixed: High

Suggested reviewers: cv, rsliter, sandl99

Merge Risk: ⚪ Minimal · up to 18b9f

E2E SDK installs now use the existing lockfile while preserving manifest and lifecycle-script protections. Both workflow paths have matching offline regression coverage, leaving no merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary change: retaining locked dependencies during E2E SDK installation by removing the package-lock override.
Linked Issues check ✅ Passed The pull request satisfies issue #11449. It removes --package-lock=false from both SDK installation paths, updates the validator, preserves the required installation controls, and adds regression cove…
Out of Scope Changes check ✅ Passed All changes support the linked issue objectives. The workflow updates, validator changes, regression tests, and workflow trigger updates are directly related to correcting E2E SDK installation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4…
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/11449-e2e-sdk-lockfile

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

@ericksoa

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-code-quality

github-code-quality Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 18b9f1f in the fix/11449-e2e-sdk-lo... branch remains at 96%, unchanged from commit 3e6fabc in the main branch.

TypeScript / code-coverage/cli

The overall line coverage in commit 18b9f1f in the fix/11449-e2e-sdk-lo... branch remains at 83%, unchanged from commit 3e6fabc in the main branch.


Updated September 10, 2026 15:37 UTC

@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure bug-fix PR fixes a bug or regression labels Sep 10, 2026
@ericksoa ericksoa closed this Sep 10, 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 bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

E2E SDK installation ignores the lockfile and fails with edgesOut

2 participants