fix(e2e): install reviewed OpenShell SDK for catalogue jobs - #11397
Merged
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@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. |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe PR integrates a reviewed OpenShell SDK artifact into catalogue E2E workflows. It adds credential-free SDK installation and validation, expands workflow boundary tests, and preserves structured CLI error details in JSON output. ChangesOpenShell SDK E2E integration
CLI JSON error serialization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SDKPackageJob
participant CatalogueJob
participant StandardProfile
participant OpenShellSDK
SDKPackageJob->>CatalogueJob: provide reviewed artifact name
CatalogueJob->>StandardProfile: pass openshell_sdk_artifact_name
StandardProfile->>SDKPackageJob: download named artifact
StandardProfile->>OpenShellSDK: install one archive without credentials or scripts
StandardProfile->>OpenShellSDK: validate OpenShellClient.connect
Suggested reviewers: ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Comment |
Contributor
13 tasks
cv
marked this pull request as ready for review
September 10, 2026 08:48
This was referenced Sep 10, 2026
sandl99
added a commit
that referenced
this pull request
Sep 10, 2026
## Outcome Catalogue E2E jobs and the external-gateway health job install the reviewed OpenShell SDK from the locked dependency graph. SDK setup no longer re-resolves project dependencies before the tests run. ## Reason [The gateway job](https://github.com/NVIDIA/NemoClaw/actions/runs/34477226195/job/102872848753) failed during the SDK install added by #11397. At inspection time, 61 jobs in that run failed at the same shared step. The `--package-lock=false` install reproduced an npm Arborist `edgesOut` crash while resolving Vitest peers, before any gateway test ran. ## Changes - Add the reviewed SDK archive to npm's cache, then use `npm ci` with lifecycle scripts disabled and cached packages preferred. Apply this to both SDK installation paths. - Keep package credentials out of cache staging, installation, and SDK validation. Require the SDK connection API to load in both workflows. - Add offline tests with real npm and local package archives. Cover the missing optional SDK, locked transitive and sibling dependencies, disabled lifecycle scripts, and setup failures. Select these tests when either owning workflow changes. ## Verification - Reproduced the original `edgesOut` crash with Node 22.23.2 and npm 10.9.8 in Linux ARM64 using the retained, integrity-verified SDK archive. - Executed the fixed catalogue script from the changed YAML against the full dependency graph with networking disabled: 509 packages installed, the SDK API loaded, and SHA-256 digests of both manifests remained unchanged. - `npx vitest run --project e2e-support test/e2e/support/openshell-sdk-install.test.ts test/e2e/support/standard-profile-workflow-boundary.test.ts test/e2e/support/base-image-publication-workflow-boundary.test.ts test/e2e/support/workflow-plan.test.ts test/e2e/support/e2e-operations-workflow-boundary.test.ts --coverage=false` — 255 tests passed. The final SDK test rerun passed all 8 tests. - `npx vitest run --project integration test/repository/vitest-watch-triggers.test.ts --coverage=false` — 77 tests passed. - `npm run checks:repository`, `npm run source-shape:check`, changed-file Oxlint/Oxfmt, ShellCheck for both install scripts, and `git diff --check` — passed. - `NODE_OPTIONS=--max-old-space-size=8192 npm run validate:pr` — passed in the isolated Linux container after building the CLI and plugin. All pre-commit, commit-message, and applicable pre-push checks passed. - Reviewed the complete diff for secrets, API keys, and credentials; none are included. ## Review notes Self-review covers NVIDIA/NemoClaw commit `316b0a6606cff97147f2c4a78ec9b49cc1f51de7`, including sensitive paths `.github/workflows/e2e-standard-profile.yaml`, `.github/workflows/e2e.yaml`, and `tools/e2e/standard-profile-workflow-boundary.mts`. Review checked archive selection, lockfile installation, credential removal, disabled scripts, failure propagation, sibling coverage, and regression evidence. Independent review of these paths is pending. Publication validation uses canonical base `5b82037acb402862bb590ef57cb4d4c608e35ce7`. Its validation entry points, hook configuration, package manifests, and lockfiles match the candidate. The changed workflow validator expectations and watch mappings were reviewed separately. Validation runs in an offline container without host credentials, signing access, host mounts, or a Docker socket. Resolved validator files and the Node/npm executables are fingerprinted. This records the maintainer-requested publication evidence and does not claim independent approval or a CI waiver. GitHub CI and live E2E confirmation remain pending. --- Signed-off-by: San Dang <sdang@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved end-to-end test setup by installing the SDK through a cached, offline-compatible workflow. - Removed the need to provide package credentials during SDK installation. - Added validation that the SDK connection API is available before tests run. - **Tests** - Expanded coverage for installation failures, invalid or missing packages, and staged setup issues. - Updated workflow monitoring so SDK installation checks run when relevant E2E workflows change. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: San Dang <sdang@nvidia.com>
prekshivyas
added a commit
that referenced
this pull request
Sep 11, 2026
## Outcome Manual PR E2E now stages the active reviewed OpenShell SDK and any approved transition replacement, then lets the candidate lockfile select the exact version. This preserves ordinary main runs while allowing reviewed SDK upgrade PRs to reach product tests. ## Reason PR #11251 run 34540154148 failed across the catalogue before product execution. The trusted workflow packaged SDK 0.0.106 from main, while the candidate lock requires 0.0.116. After #11455 switched installation to cache staging plus npm ci, npm correctly followed the candidate lock but could not satisfy it from the single cached archive, omitted the private optional dependency, and every consumer failed with ERR_MODULE_NOT_FOUND. #11397 introduced the single-main-version artifact assumption. #11455 exposed that latent mismatch deterministically. ## Changes - Add an optional reviewed-replacement packaging mode without weakening the existing strict transition mode used by pull_request_target. - Make E2E package the active SDK plus an available base-approved replacement. - Allow exactly one archive in steady state or two during an approved transition, cache each without credentials or lifecycle scripts, and retain lockfile-driven npm ci. - Add real-npm coverage for both active and replacement lock selection in catalogue and external-gateway workflows. - Extend workflow and package-verifier boundary coverage and documentation. ## Verification - Exact retained PR artifact from run 34536766601 contained reviewed SDK 0.0.106 and 0.0.116 archives. Against PR #11251 package.json and package-lock.json, the proposed commands installed 509 packages, selected exactly SDK 0.0.116, and imported OpenShellClient.connect. - Real Linux npm tests cover active and replacement selection in both workflow scripts: 11/11 passed. - Package transition tests: 5/5 passed. - Standard-profile boundary tests: 17/17 passed. - Base-image, operations, watch-trigger, and workflow-plan coverage: 320 tests passed initially; the 19 missing-esbuild setup failures were repaired, then 106/107 workflow-plan tests passed and the sole 5-second container timeout passed with a 15-second harness budget. - Repository checks, source-shape checks, canonical CLI build, TypeScript checks, formatting, lint, YAML validation, secret scan, E2E semantic phases, growth guardrails, commit hooks, and pre-push hooks passed. - Brev was attempted first but its local auth-token JSON is truncated; a credential-free feedback report was submitted. The equivalent Linux Docker reproduction completed locally. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * SDK packaging and installation now support one or two approved archive versions, including available transition replacements. * Optional replacement packages can be included when available, while required replacements are validated explicitly. * **Bug Fixes** * Archive discovery is now deterministic and rejects missing, excessive, or ambiguous archive combinations. * Reviewed SDK archives are cached offline without package credentials. * Dependency installation verifies the selected SDK version and suppresses lifecycle scripts for safer setup. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Outcome
Catalogue E2E jobs install the reviewed OpenShell SDK before running SDK-backed commands such as
nemoclaw config export. JSON command failures also retain their redacted error message alongside exit metadata.Reason
The network-policy job failed during configuration export, after onboarding and before its network-policy probes. Its retained output contained only
error.oclif.exit, which hid the failure reason.Catalogue workspace setup uses
npm ci --ignore-scriptswithout package credentials. The private OpenShell SDK is an optional dependency, so that setup can succeed without installing the SDK required by configuration export. The workflow already packages a verified SDK archive for external-gateway health checks, but catalogue jobs did not consume it.Changes
OpenShellClient.connectloads before restoring the CLI and running the target.Error.messagebefore the existing JSON redaction. Add command and configuration-export regressions.Verification
npx vitest run --project cli --project e2e-support src/commands/config/export.test.ts src/lib/cli/nemoclaw-oclif-command.test.ts src/commands/doctor.test.ts test/e2e/support/standard-profile-workflow-boundary.test.ts test/e2e/support/openshell-sdk-install.test.ts test/e2e/support/base-image-publication-workflow-boundary.test.ts test/e2e/support/workflow-plan.test.ts --coverage=false.npm run build:cliandnpm run validate:pr: passed for publication commitce894e7594cec7d63c1c490ebcbb75d556447aaain the isolated checkout.npm run check: 34,442 CLI/integration tests passed, 280 failed, and one suite failed during TLS fixture setup; plugin coverage passed. Rerunning all 45 failing files on canonical basee94c06f130e1904229c9214f9d1f914b98f6b1bareproduced the same 281 failure records. These include temporary-checkout permissions, OpenSSL fixtures, process cleanup in the container, and DGX Spark host detection. This local broad gate is not reported as passing.Review notes
Self-review covers NVIDIA/NemoClaw commit ce894e7, including sensitive paths
.github/workflows/e2e.yaml,.github/workflows/e2e-standard-profile.yaml,tools/e2e/operations-workflow-boundary.mts, andtools/e2e/standard-profile-workflow-boundary.mts. Review checked the package producer, all catalogue consumers, credential boundaries, failure behavior, and regression evidence. No independent pre-publication review has occurred; these paths await independent review in this draft PR.Validation uses canonical base
e94c06f130e1904229c9214f9d1f914b98f6b1ba. Package manifests, lockfiles, hook configuration, and validator entry points match that base. Because the E2E validation helpers change, checks ran in a disposable Ubuntu container with no network, host credentials, signing agent, Docker socket, or added capabilities. Node and resolved validator executables were fingerprinted, and the tested source tree was checked against the publication tree. Publication hooks run in that same isolation and propagate failures; commit signing remains outside the container. This records validation for the maintainer-requested PR and does not claim independent approval or a CI waiver.Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
Bug Fixes
Security
Documentation
Tests