ci: finish executable workflow extraction - #7709
Merged
cv merged 10 commits intoJul 28, 2026
Merged
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv
added
area: ci
Contributor
|
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 extracts WSL provisioning and execution into a tested PowerShell helper, updates platform Vitest and WSL E2E workflows to use trusted helper revisions, strengthens workflow boundary validation, and adds helper and trigger coverage. ChangesWSL CI extraction
Workflow integration and validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant TrustedHelper
participant WSL
Workflow->>TrustedHelper: Load helper from trusted revision
Workflow->>TrustedHelper: Ensure distro and install dependencies
TrustedHelper->>WSL: Provision Ubuntu and Node.js
Workflow->>TrustedHelper: Sync checkout and invoke test script
TrustedHelper->>WSL: Run Vitest or E2E command
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit d6662d3 in the TypeScript / code-coverage/cliThe overall coverage in commit d6662d3 in the 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tools/wsl/ci-helper.ps1 (1)
53-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStatic-analysis naming warnings vs. cross-file naming contract.
PSScriptAnalyzer flags
Ensure-WslDistro(unapproved verb) andNew-WslScriptArguments/Set-WslWorkflowPaths/Install-WslUbuntuDependencies(plural nouns), plus missingShouldProcesssupport on state-changing functions. Renaming these isn't free here: bothplatform-vitest-main.yamlandwsl-e2e.yamlalready call these exact names (Ensure-WslDistro,Set-WslWorkflowPaths,Install-WslUbuntuDependencies,Sync-WslCheckout), so a rename is a breaking cross-file change. Consider suppressing these specific rules for this file instead.♻️ Suggested suppression
+#Requires -Version 5.1 +[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseApprovedVerbs', '', Scope = 'Function', Target = 'Ensure-WslDistro', Justification = 'Name is a stable cross-workflow contract')] +[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns', '', Scope = '*', Justification = 'Names are a stable cross-workflow contract')] +param() Set-StrictMode -Version LatestAlso applies to: 147-147, 178-178, 275-275
🤖 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 `@tools/wsl/ci-helper.ps1` at line 53, Suppress the specific PSScriptAnalyzer naming and ShouldProcess warnings for this file rather than renaming the cross-file functions. Preserve the existing names Ensure-WslDistro, New-WslScriptArguments, Set-WslWorkflowPaths, Install-WslUbuntuDependencies, and Sync-WslCheckout so platform-vitest-main.yaml and wsl-e2e.yaml continue to resolve them.Source: Linters/SAST tools
🤖 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 @.github/workflows/wsl-e2e.yaml:
- Around line 66-70: Update the candidate checkout step using actions/checkout
in the workflow to set persist-credentials to false, matching the trusted helper
and sibling candidate checkout configuration while preserving the existing
source path.
- Around line 175-193: Update the WSL script execution block around
Invoke-WslScript to remove the generated RUNNER_TEMP\nemoclaw-wsl-step.sh file
in a finally block, ensuring cleanup runs on both success and failure. Preserve
restrictive permissions for the temporary script and use the existing
temporary-file path rather than introducing another location.
In `@tools/e2e/operations-workflow-boundary.mts`:
- Around line 544-552: Update the NEEDS_JSON validation in the report-to-pr
boundary checks to match the complete environment-variable and interpolation
expressions rather than unsafe substrings. Reject equivalent whitespace variants
of the toJSON(needs) interpolation and similarly named variables such as
NEEDS_JSON_BAD, using syntax-aware or boundary-matched checks; apply the same
fix to both validation blocks and add regression cases covering these variants.
---
Nitpick comments:
In `@tools/wsl/ci-helper.ps1`:
- Line 53: Suppress the specific PSScriptAnalyzer naming and ShouldProcess
warnings for this file rather than renaming the cross-file functions. Preserve
the existing names Ensure-WslDistro, New-WslScriptArguments,
Set-WslWorkflowPaths, Install-WslUbuntuDependencies, and Sync-WslCheckout so
platform-vitest-main.yaml and wsl-e2e.yaml continue to resolve them.
🪄 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: 7679ac02-4a91-43c3-b04d-b82c242fb2c9
📒 Files selected for processing (11)
.github/workflows/e2e.yaml.github/workflows/platform-vitest-main.yaml.github/workflows/wsl-e2e.yamlci/source-shape-test-budget.jsontest/e2e/support/e2e-operations-workflow-boundary.test.tstest/helpers/vitest-watch-triggers.tstest/platform-vitest-main-workflow.test.tstest/vitest-watch-triggers.test.tstest/wsl-ci-helper.test.tstools/e2e/operations-workflow-boundary.mtstools/wsl/ci-helper.ps1
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
23 tasks
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
19 tasks
cv
added a commit
that referenced
this pull request
Jul 28, 2026
<!-- markdownlint-disable MD041 --> ## Summary This replacement stacked PR completes #6952 by extracting the remaining repeated Windows and WSL setup and command construction. It uses the trusted helper from #7719 and restores the reviewed #7709 diff after #7709 merged into its staging branch. ## Related Issue Fixes #6952 Fixes #6958 Replaces staged PR #7709 Depends on #7719 ## Changes - Use `tools/wsl/ci-helper.ps1`, bootstrapped by #7719, for WSL E2E and platform Vitest workflows. - Load the helper through mandatory sparse checkouts at the PR base SHA or workflow SHA before candidate checkout, and fail closed. - Pass `needs` through `NEEDS_JSON` and parse it instead of interpolating expressions into executable code. - Reduce inline executable workflow logic while retaining root-only selection and its reason. - Reapply the exact reviewed #7709 patch after recovering #7719's helper-only scope. The binary patch is identical to `df3bcdcdd`. ## Type of Change - [x] Code - [ ] Documentation - [ ] Tests only - [ ] Other ## Quality Gates - [x] Tests added or updated for changed behavior - [x] Documentation is not applicable because this changes internal CI only - [x] Sensitive paths changed - [x] Sensitive-path review completed: the exact `c1f34675d1025b7f189938977cf6158efdc66b4c` patch is byte-for-byte identical to reviewed #7709 commit `df3bcdcdd`; targeted boundary review covers trusted provenance, fail-closed loading, bidirectional overlap, quoting, encoding, `needs` interpolation, and root/non-root separation - [ ] Non-success behavior changed ## Documentation Writer Review - [x] Documentation writer review completed Result: no-docs-needed Evidence: Reviewed exact head `c1f34675d1025b7f189938977cf6158efdc66b4c` against base `e69d9c3d94623a0cd499fb2c84f33f88c863c631`. The expected nine files match the binary patch SHA-256 for reviewed #7709 commit `df3bcdcdd`; no user-facing documentation changes are required. Focused tests passed 50 cases with 12 PowerShell cases skipped because PowerShell is unavailable locally. CLI type-check and exact-base diff checks passed. Agent surface: Codex Desktop <!-- docs-review-head-sha: c1f3467 --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Test Evidence Not applicable. ## Verification - [x] DCO declaration is included and every commit is GitHub Verified - [x] Normal commit and pre-push hooks passed - [x] Targeted tests passed: 50 passed; 12 PowerShell cases skipped locally because PowerShell is unavailable - [x] CLI type-check and exact-base diff checks passed - [ ] Broad test suite passed - [x] Quality gates above are complete - [x] No secrets, API keys, or credentials are committed - [ ] User-facing documentation was updated - [ ] Documentation build passed Signed-off-by: Carlos Villela <cvillela@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv
added a commit
that referenced
this pull request
Jul 28, 2026
<!-- markdownlint-disable MD041 --> ## Summary Add the tested WSL provisioning helper to a trusted `main` revision before consumer workflows adopt it. This helper-only PR is the first stage of #6958 so root-capable WSL execution can fail closed until the helper is present on `main`. ## Related Issue Part of #6958 Parent: #6952 Previous staged extraction: #7709 (merged into this branch and reverted) Previous replacement extraction: #7737 (merged into this branch and reverted) Next step: recreate the extraction directly from `main` after this PR lands ## Changes - Add `tools/wsl/ci-helper.ps1` for WSL path conversion, LF-safe script transfer, command construction, distro provisioning, Node.js installation, and ext4 checkout synchronization. - Add direct PowerShell batch tests for quoting, spaces, drive-letter conversion, LF encoding, optional-argument binding, deletion-path restrictions, install retry behavior, exit propagation, and temporary-script cleanup on success and failure. - Bootstrap the helper separately because #6958 requires the helper to be trusted before root-capable WSL execution. A combined first change cannot satisfy that requirement because its pull-request base does not yet contain the helper. - Preserve the audit trail with signed, non-force reverts after #7709 and #7737 were merged into this staging branch. The resulting PR diff is helper-only again. ## 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 adds an internal CI helper and direct tests. Consumer workflow adoption will be recreated in a separate PR after this helper lands, so no user-facing or contributor-facing contract changes here. - [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: Codex security review at `3a5ebeb2f` passed all nine categories, and the helper files at `92507d621` are byte-for-byte unchanged. The helper keeps WSL arguments separate and quotes shell values. Sync deletion is limited to one positive run-ID/attempt child under the two workflow-owned roots and fails closed for symlinked roots, bidirectional checkout overlap, traversal, and unrelated paths. Package and owner inputs are validated, pinned Node.js archives are verified, transferred scripts are deleted in `finally`, and direct tests protect these boundaries. - [ ] 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`; writing review passed with no findings - Evidence: At exact head `92507d621762bb77ac7e76baf50d9a7c6ee87f28` and base `bbdd85008c34503ea47d4a80d57e90074ff3c79e`, the exact-base diff adds only `tools/wsl/ci-helper.ps1` and `test/wsl-ci-helper.test.ts`. Both match the previously reviewed helper tree. No documentation sources or user-facing contracts changed. Twelve PowerShell cases were discovered and skipped because PowerShell is unavailable locally; source architecture checks, CLI type-check, normal hooks, and diff check passed. - Agent: Codex Desktop <!-- docs-review-head-sha: 92507d6 --> <!-- 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 - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: The focused integration command collected all twelve direct PowerShell cases and skipped them because PowerShell is unavailable locally; PR CI runs them on a PowerShell-capable shard. `npm run checks`, `npm run typecheck:cli`, normal hooks, and the diff check passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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 a Windows-to-WSL CI helper that generates Ubuntu provisioning and checkout-sync scripts, provisions apt dependencies, and installs a pinned Node.js version in the target distro. * Added strict Windows path conversion to `/mnt/...`, safe bash literal escaping, and LF-only UTF-8 script writing without a byte-order mark. * Improved WSL distro install robustness with bounded retries and safe checkout workdir validation. * **Tests** * Added Vitest coverage for conversions, WSL argument construction, generated scripts, distro retry recovery, workdir rejection, exit propagation, and temporary-script cleanup. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
23 tasks
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
This stacked PR completes #6952 by extracting the remaining repeated Windows/WSL setup and command construction. It uses the trusted PowerShell helper bootstrapped by #7719, switches both workflow consumers to that trusted boundary, and fails closed if the helper is absent. It also passes the
needscontext through environment data instead of interpolating it into GitHub Script source.Related Issue
Fixes #6952
Fixes #6958
Changes
tools/wsl/ci-helper.ps1, bootstrapped by ci(wsl): bootstrap trusted workflow helper #7719, for the WSL E2E and platform Vitest workflows. Both consumers require the same distro setup, script transfer, Node.js installation, and ext4 checkout synchronization. Keeping this logic inline duplicated the security-sensitive command boundary; the helper and workflow regression tests protect the shared behavior and connection.needsJSON throughNEEDS_JSONand parse it inside GitHub Script. The E2E operations boundary rejects direct expression interpolation.wsl-e2e.yamland from 228 to 129 inplatform-vitest-main.yaml. Keep the root-only test selection inline with an adjacent rationale.Type of Change
Quality Gates
d6662d367, targeted boundary review and tests verify trusted helper provenance, fail-closed loading, bidirectional checkout/workdir overlap rejection, command quoting, script encoding, credential-data interpolation, and root/non-root separation.Documentation Writer Review
no-docs-neededd6662d367, the exact-base diff against helper head3a5ebeb2fcontains the expected nine internal workflow, validator, trigger, and regression-test files. The reviewed overlap guard is present in both head and base, so helper files do not appear in this PR diff. No documentation sources changed; focused tests, CLI type-checking, and the exact-base diff check passed.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 run typecheck:cli, the exact-base diff check, normal commit hooks, and the pre-push TypeScript check 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: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
needsis provided viaenvand parsed as JSON (no inline interpolation).