test(e2e): retire local-only scenarios - #7625
Merged
Merged
Conversation
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 (30)
📝 WalkthroughWalkthroughThis change retires several local-only E2E jobs and mappings, converts selected checks to direct Vitest or compiled-CLI contract tests, routes installer coverage to ChangesE2E scenario retirement
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 7798aa7 in the TypeScript / code-coverage/cliThe overall coverage in commit 7798aa7 in the Show a code coverage summary of the most impacted files.
Updated |
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported 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: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv
marked this pull request as ready for review
July 27, 2026 16:16
23 tasks
cv
added a commit
that referenced
this pull request
Jul 27, 2026
<!-- markdownlint-disable MD041 --> ## Summary PR #7625 correctly retired six local-only E2E scenarios, but the trusted controller on `main` can still dispatch those retired selector IDs while a candidate workflow is taking over. This adds a fail-closed transition bridge: the candidate runs the ordinary replacement tests once and emits controller-bound evidence for each retired selector while the remaining live jobs continue. ## Related Issue Follow-up to #7625 and #7616 Parent: #7614 ## Changes - Add a compatibility job for the current requirement that trusted PR E2E controllers can select IDs retired by the candidate they dispatch. The consumer is the SHA-bound `e2e.yaml` workflow invoked by the controller on `main`. - Keep the raw trusted selector list for evidence, filter only the six known retired IDs from candidate planning, and fail closed when no active controller-selected job remains. - Run each replacement Vitest project once, require the retired live files to remain absent, and emit SHA-, plan-, correlation-, and shard-bound risk signals for all selected retired IDs. - Extend the prepare/upload workflow boundary contracts and add planner and compatibility regression tests. A direct controller change is insufficient because the controller must remain on trusted `main`; restoring the retired live jobs would reintroduce the cost this migration removes. - Keep the retired selector list authoritative in the executable workflow boundary, reject workflow-gate drift, and pass the caller-supplied environment to every replacement command. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: This is an internal controller-transition path. It adds no supported CLI behavior, configuration, operator action, or public documentation contract. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: The bridge remains SHA-, plan-, correlation-, and shard-bound, rejects unsafe selectors, preserves raw trusted evidence, and fails closed without another active controller job. The exact nine-selector replay emitted six valid bound signals. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: No documentation paths changed or require updates. The reviewer checked `docs/`, `README.md`, `CONTRIBUTING.md`, and `test/e2e/README.md`; existing guidance already covers controller planning, SHA-bound checkout, protected approval, and evidence validation without promising transient selector IDs. - Agent: Codex Desktop documentation-writer subagent (`/root/documentation_writer_review`) <!-- docs-review-head-sha: 8980c62 --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## 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 — exact nine-selector replay: 47 integration, 3 installer-integration, and 4 package-contract assertions passed with six bound risk signals; focused workflow contracts: 75/75; full E2E-support: 163 files passed, 3 skipped, 1,737 tests passed, 17 skipped; source-shape cases remain zero; semantic phase coverage: 117 tests across 76 files; Vitest membership: exact 1,861 files across 8 projects; `npm run build:cli` and `npm run check:diff` passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: `npm test` was attempted on the original PR head and reproduced three unchanged DGX Station host-detection fixture failures plus one unchanged root-topology fixture failure, then stopped making progress and was interrupted. GitHub CI will provide hosted aggregate evidence for the updated head. - [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) - [ ] 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: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added retired-selector-compatibility checks that run targeted replacement E2E tests and generate compatibility evidence artifacts (including risk-signal and summary output). * Expanded CI and PR reporting to include a dedicated retired-selector-compatibility job when relevant. * Updated workflow planning and boundary validation to map controller-selected jobs to compatible sets and enforce retired-only selection rules. * **Bug Fixes** * Prevented planning/runs when controller-selected jobs become fully retired without required targets. * **Tests** * Added E2E coverage for selector filtering, artifact outputs, error/guard behaviors, and workflow selector gate drift detection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
23 tasks
wscurran
added
area: ci
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
2 participants
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.
Summary
Retire six Priority 0 E2E scenarios whose checks do not require live cloud state. Their assertions now run in ordinary integration or installer tests, or remain covered by dedicated PR workflows, reducing the shared credential-free matrix from six tests to one and removing four standalone workflow jobs.
Based on recent job medians, this removes about 5.7 runner-minutes from a full E2E invocation without weakening the covered behavior.
Related Issue
Fixes #7616
Parent: #7614
Changes
Type of Change
Quality Gates
Documentation Writer Review
No documentation update required/root/documentation_writer_review)DGX Station Hardware Evidence
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 unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm testwas attempted with the SSH signing agent. The aggregate run was environment-limited by three unchanged DGX Station host-OS cases and one unchanged root-topology fixture (exec /usr/local/bin/python3: exec format error), then stopped after Vitest ceased producing output. GitHub CI is pending.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Refactor