fix(ci): run selected typed targets in PR E2E#7046
Conversation
Signed-off-by: Carlos Villela <cvillela@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:
📝 WalkthroughWalkthroughThe PR adds allowlisted typed-target selection across E2E risk plans, workflow dispatch, PR gate state, evidence validation, controller orchestration, tests, and maintainer documentation. Jobs and typed targets can now be dispatched and evaluated together. ChangesTyped-target E2E gate
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PRController
participant RiskPlan
participant E2EWorkflow
participant LiveVitest
participant PRGate
PRController->>RiskPlan: Build jobs and typed-target requirements
RiskPlan->>PRController: Required selections and plan hash
PRController->>E2EWorkflow: Dispatch jobs and approved targets
E2EWorkflow->>LiveVitest: Run generated matrix target
LiveVitest->>PRGate: Publish risk-signal evidence
PRGate->>PRController: Classify evidence and update gate
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. Updated |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md:
- Line 39: The E2E control-plane automatic-dispatch boundary is missing
tools/e2e/pr-e2e-required.mts. Update the “E2E control-plane authorization”
description in MERGE-GATE.md to include that file alongside
.github/workflows/pr-e2e-gate.yaml and tools/e2e/pr-e2e-gate.mts, preserving the
existing trusted-boundary behavior.
In @.github/workflows/e2e.yaml:
- Around line 164-171: Validate the matrix produced by workflow-plan.mts at the
controller-dispatch boundary before launching any credential-bearing jobs. For
controller dispatches, require the sorted matrix[].id set to exactly match
TARGETS, including an empty matrix when TARGETS is empty, and reject any
PR-generated or unallowlisted target. Add the same invariant coverage to the
workflow-boundary tests.
In `@test/pr-e2e-gate-typed-target.test.ts`:
- Around line 58-94: Append the required local issue reference as a final
“(`#1234`)” suffix to the parent describe title in
test/pr-e2e-gate-typed-target.test.ts. Also update test/pr-e2e-gate.test.ts at
the cited site by appending the same suffix to its title or ensuring its
enclosing describe provides it.
🪄 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: 97f2e5b4-24d1-47ba-ac20-de6ade7c1b44
📒 Files selected for processing (19)
.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md.github/workflows/e2e.yamltest/e2e/README.mdtest/e2e/support/e2e-workflow.test.tstest/e2e/support/workflow-plan.test.tstest/pr-e2e-gate-fork-skip.test.tstest/pr-e2e-gate-lifecycle.test.tstest/pr-e2e-gate-typed-target.test.tstest/pr-e2e-gate-workflow.test.tstest/pr-e2e-gate.test.tstest/pr-risk-plan.test.tstools/advisors/e2e-recommendations.mtstools/advisors/risk-plan.mtstools/e2e/operations-workflow-boundary.mtstools/e2e/pr-e2e-gate.mtstools/e2e/upload-e2e-artifacts-workflow-boundary.mtstools/e2e/workflow-boundary.mtstools/e2e/workflow-plan.mtstools/pr-review-advisor/analyze.mts
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence 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
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>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/support/e2e-report-to-pr-workflow-boundary.test.ts (1)
82-82: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse a POSIX
:separator for this Bash test.Line 82 uses
path.delimiter, which becomes;under Windows Node even though the generated script runs under Bash and requires:. Use${binDirectory}:${process.env.PATH ?? ""}to keep the fakenpxdiscoverable.Based on learnings, this repository’s tests use the POSIX
:separator because CI runs on Linux/WSL.🤖 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/e2e-report-to-pr-workflow-boundary.test.ts` at line 82, Update the PATH assignment in the test environment setup to use a literal POSIX colon separator instead of path.delimiter, ensuring the generated Bash script can discover the fake npx executable. Preserve the existing binDirectory prefix and empty-PATH fallback.Source: Learnings
🤖 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/e2e-report-to-pr-workflow-boundary.test.ts`:
- Line 82: Update the PATH assignment in the test environment setup to use a
literal POSIX colon separator instead of path.delimiter, ensuring the generated
Bash script can discover the fake npx executable. Preserve the existing
binDirectory prefix and empty-PATH fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 879deb41-2046-403c-96ab-a7995b347277
📒 Files selected for processing (8)
.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md.github/workflows/e2e.yamltest/e2e/README.mdtest/e2e/support/e2e-report-to-pr-workflow-boundary.test.tstest/e2e/support/e2e-workflow.test.tstest/pr-e2e-gate-typed-target.test.tstest/pr-e2e-gate.test.tstools/e2e/workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (6)
- test/e2e/support/e2e-workflow.test.ts
- test/pr-e2e-gate-typed-target.test.ts
- .github/workflows/e2e.yaml
- test/e2e/README.md
- test/pr-e2e-gate.test.ts
- tools/e2e/workflow-boundary.mts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
Maintainer exact-head update for the landing candidate:
The prior approval is attached to parent head |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
The first credentialed gate for I pushed signed, Verified commit Please attach/confirm human approval on exact head |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary This PR upgrades every stable NemoClaw OpenShell selector and consumed artifact from `0.0.72` to stable `0.0.85`, including the #6379 credential-rewrite fix and OpenShell's multiline exec contract. Stable `v0.0.85` is verified at [`3dee5570a46076a57a3b056f35f35ebc0861ac85`](NVIDIA/OpenShell@3dee557). Base-owned structured trust landed in #6778 and #6779, the exact `v0.0.85` manifest identities landed separately in #7069, and the controller-compatible legacy gateway evidence matrix landed in #7055. The branch is refreshed on current `main`. Exact-head GitHub CI, automated review, protected runtime proofs, and platform-specific acceptance are running or pending below. ## Related Issue Fixes #6379. ## Changes - [x] Audit all 13 adjacent OpenShell ranges from `v0.0.72` through `v0.0.85`: 67 commits and 283 changed paths, including the unpublished `v0.0.84` source boundary. Release notes were treated as leads rather than proof. - [x] Verify stable tag `v0.0.85` at `3dee5570`, successful producer run [`29507522595`](https://github.com/NVIDIA/OpenShell/actions/runs/29507522595), and inclusion of required credential fix `40194f93`. - [x] Advance the blueprint floor/ceiling, installer floor/ceiling/dev floor, Brev defaults, stable E2E gateway-auth pin, supervisor index, sandbox-build fallback, credential manifest, current docs, and user-visible messages to `0.0.85`. - [x] Pin all eight CLI/gateway/sandbox release archives, three component manifests, extracted release-binary identities, and the multi-arch supervisor image to immutable `v0.0.85` identities. Historical `0.0.72` and `0.0.82` references remain only where cleanup, fallback, trust, or compatibility evidence requires them. - [x] Retain both `v0.0.82` standalone sandbox digest fallbacks when host-side `--version` cannot run, and exercise the true `0.0.86` above-maximum boundary. - [x] Replace the migration ledger with `docs/security/openshell-0.0.85-migration-review.md`, covering the complete range, dependency delta, consumed contracts, evidence-backed exclusions, mitigations, and blocking runtime/release gates. - [x] Reject OpenShell's revisioned credential-name namespace before provider mutation while preserving cleanup of persisted legacy entries across host and Hermes adapters. - [x] Report OpenShell's fail-closed `CONNECT 503` as unavailable TLS credential rewriting instead of a generic curl transport error. - [x] Preserve LF, CR, CRLF, quotes, and heredocs byte-exactly across multiline exec while retaining NUL, multiline-workdir, and request-environment boundaries. - [x] Reject supervisor-only `OPENSHELL_TLS_CA`, `OPENSHELL_TLS_CERT`, and `OPENSHELL_TLS_KEY` if an entrypoint, exec, or connect child receives them. - [x] Validate that each selected OpenShell archive contains exactly one expected regular binary before extraction; reject absolute paths, traversal, duplicates, extras, links, and devices. - [ ] Prove the exact head on the supported Linux x86, macOS Docker Desktop/Colima, WSL, Colossus, and DGX Spark paths selected by CI/advisor. - [ ] On a physical Docker 27 DGX Spark arm64 host, prove honest `mcp status`, real credential substitution, an authenticated MCP tool call, rotation/rebuild/removal behavior, and absence of literal `openshell:resolve:env:` leakage. - [ ] Record the platform decision for Docker 27 versus upstream OpenShell's Docker 28 requirement. ## Release provenance - All eight consumed release archives have valid GitHub SLSA attestations bound to source commit `3dee5570`; the three published checksum manifests and all ten installer/Brev references were independently reverified after the update. - The selected supervisor image is the immutable multi-arch index `sha256:f4226253a3525c3832adac5b38b419a0f27d1e915effe565b5885e20f93cd5e9`. - GitHub currently exposes no OCI referrer/attestation/SBOM for that supervisor image, and its configs contain no source/revision/version labels. The review records that upstream provenance limitation rather than inferring missing evidence. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] 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: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: pending exact-head maintainer review - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes DCO sign-off declarations and all 78 commits currently appear as `Verified` in GitHub - [x] Normal commit/push hooks and `npm run check:diff` passed; the generated-platform citation repair is signed, and current exact head `610f7cd9ff9cc8d3fdc47f40271d0b56b452a0cc` is GitHub-verified - [x] The checker is byte-identical to current `main`; the base-owned `scripts/check-installer-hash.sh` passed all three manifests and all ten consumed release references against published `v0.0.85` assets - [x] Focused current-head repairs passed: feature gate 15/15, installer integration 47 passed with 1 intentional skip, installer-hash and migration ledger 78/78, gateway workflow boundary 4/4, PR E2E controller 27/27, and generated platform docs 19/19 - [x] Broader compatibility, MCP boundary, Hermes, and E2E-support tests passed earlier in the migration; the opt-in version-pin live suite passed all three `gh` and curl paths - [x] The #7046 conflict refresh preserves all four gateway-upgrade shards and the typed DCode target; combined integration passed 172/172, serial E2E-support passed 1,156 with 2 intentional skips, and file-size/project-membership checks passed - [x] `npm run docs` completed successfully, including generated variants, route validation, and Fern checks (0 errors; 2 existing warnings) - [ ] Exact-head GitHub CI, automated review, and protected E2E passed — ordinary CI, both advisor lanes, and protected E2E are running or pending at head `610f7cd9ff9cc8d3fdc47f40271d0b56b452a0cc` - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [x] 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) — the security review has the required SPDX header and is not a Fern page. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Charan Jagwani <cjagwani@nvidia.com>
Summary
PR E2E risk plans now select the exact Deep Agents Code typed target when its headless-inference check changes, so #7032 can receive its required live regression instead of only the control-plane floor jobs. The controller dispatches jobs and that target in one bound run while preserving exact-SHA authorization, fork, stale-revision, and secret-scope boundaries.
Changes
requiredTargetsfor the current Deep Agents Code check-to-target requirement. The PR gate consumes this field because a free-standing workflow job cannot address the registry target;pr-risk-plan.test.tsandpr-e2e-gate-typed-target.test.tsprotect the mapping and evidence contract.ubuntu-repo-cloud-langchain-deepagents-codetarget. The trusted workflow independently rejects unapproved controller target input and requires the generated matrix IDs to exactly match that selection before exposing any matrix output or starting credential-bearing work.matrix.id, upload itsdefault-shard signal, and fail closed for missing, duplicate, skipped, malformed, stale, or unauthorized evidence.Type of Change
Quality Gates
run-control-planeauthorization, forks and stale revisions do not dispatch, and the NVIDIA inference secret remains scoped to the existing live-test step.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 run test:changed: 208 passed;npm run build:cli && npm run typecheck:cli: passed. After binding the generated matrix to the controller selection: focused controller integration: 142 passed; workflow/E2E support: 113 passed;npm run typecheck:cli: passed. CodeRabbit PATH follow-up: workflow-boundary test 31 passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Focused control-plane contracts and the changed-test lane were used for this narrow prerequisite.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
Signed-off-by: Charan Jagwani cjagwani@nvidia.com