ci: finish executable workflow extraction - #7737
Merged
cv merged 2 commits intoJul 28, 2026
Merged
Conversation
<!-- markdownlint-disable MD041 --> ## 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 `needs` context through environment data instead of interpolating it into GitHub Script source. ## Related Issue Fixes #6952 Fixes #6958 ## Changes - Use `tools/wsl/ci-helper.ps1`, bootstrapped by #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. - Load the helper through mandatory sparse checkouts at the PR base SHA or workflow SHA before the candidate checkout. Both workflows fail closed if the trusted helper is missing; the workflow boundary tests protect the trusted-source requirement. - Pass `needs` JSON through `NEEDS_JSON` and parse it inside GitHub Script. The E2E operations boundary rejects direct expression interpolation. - Reduce nonblank executable lines from 191 to 80 in `wsl-e2e.yaml` and from 228 to 129 in `platform-vitest-main.yaml`. Keep the root-only test selection inline with an adjacent rationale. ## 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 changes internal CI workflows and their tests. It does not change a CLI, configuration, supported integration, or end-user 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: At stacked PR SHA `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. - [ ] 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: At exact head `d6662d367`, the exact-base diff against helper head `3a5ebeb2f` contains 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. - Agent: Codex Desktop <!-- docs-review-head-sha: d6662d3 --> <!-- 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 — command/result or justification: The combined focused run passed 50 tests across the platform workflow, watch-trigger, and E2E boundary suites. It also discovered twelve PowerShell helper tests and skipped them locally because PowerShell is unavailable. `npm run typecheck:cli`, the exact-base diff check, normal commit hooks, and the pre-push TypeScript 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 * **Bug Fixes** * Improved reliability and orchestration of WSL-based test runs with clearer conditional execution and safer setup/sync. * **Security** * Strengthened workflow-boundary enforcement so `needs` is provided via `env` and parsed as JSON (no inline interpolation). * Routed WSL provisioning/execution through a trusted helper path. * **Tests** * Expanded CI, boundary, and trusted-helper suite coverage for script generation, path handling, and failure propagation. * Updated workflow trigger mappings to include the trusted helper. * **Chores** * Consolidated WSL workflow logic into the shared trusted helper flow. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
23 tasks
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. |
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
cv
added a commit
that referenced
this pull request
Jul 28, 2026
<!-- markdownlint-disable MD041 --> ## Summary This PR completes #6952 and #6958 by moving repeated Windows and WSL executable logic into the trusted helper from #7719. It recreates the #7737 extraction directly on `main` after the helper landed. ## Related Issue Fixes #6952 Fixes #6958 ## Changes - Use `tools/wsl/ci-helper.ps1` for WSL E2E and platform Vitest setup, script transfer, Node.js installation, and ext4 checkout synchronization. The workflow regression tests protect both consumers. - Load the helper from the PR base SHA or workflow SHA before candidate checkout. Both workflows fail if the trusted helper is missing. - Pass `needs` through `NEEDS_JSON` instead of interpolating GitHub expressions into executable source. The E2E operations boundary test rejects direct interpolation. - Validate through the TypeScript AST that `needs` receives the parsed environment value. Isolated tests reject comments, unrelated assignments, and malformed wiring. - Update watch triggers and the source-shape budget for the extracted logic. ## 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 PR changes internal CI workflows and tests. It does not change a CLI, configuration, supported integration, or user-facing 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 trusted helper is loaded from the base or workflow SHA before candidate checkout. Focused tests verify trusted provenance, failure when the helper is missing, path overlap rejection, quoting, encoding, `needs` assignment and interpolation, and root separation. - [ ] 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: Reviewed head `b56897529e166385464c7e8ec051174f3af4f2aa` against base `c3ab0526eba41d1c681e92bbad3d174925aeaa6a`. The AST validator and its regressions change internal CI security enforcement only. No documentation source or user-facing contract changed. The review-fix suite passed 45 tests. Biome, repository checks, CLI type-checking, the diff check, source-shape budget, title check, gitleaks, and commitlint passed. - Agent: Codex Desktop <!-- docs-review-head-sha: b568975 --> <!-- 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 — command/result or justification: The original focused suite passed 50 tests and skipped 12 PowerShell-only tests. After the review fix, the E2E operations boundary suite passed 45 tests. `npx biome check` for the changed files, `npm run checks`, `npm run typecheck:cli`, and `git 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> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
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
3 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 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
tools/wsl/ci-helper.ps1, bootstrapped by ci(wsl): bootstrap trusted workflow helper #7719, for WSL E2E and platform Vitest workflows.needsthroughNEEDS_JSONand parse it instead of interpolating expressions into executable code.df3bcdcdd.Type of Change
Quality Gates
c1f34675d1025b7f189938977cf6158efdc66b4cpatch is byte-for-byte identical to reviewed ci: finish executable workflow extraction #7709 commitdf3bcdcdd; targeted boundary review covers trusted provenance, fail-closed loading, bidirectional overlap, quoting, encoding,needsinterpolation, and root/non-root separationDocumentation Writer Review
Result: no-docs-needed
Evidence: Reviewed exact head
c1f34675d1025b7f189938977cf6158efdc66b4cagainst basee69d9c3d94623a0cd499fb2c84f33f88c863c631. The expected nine files match the binary patch SHA-256 for reviewed #7709 commitdf3bcdcdd; 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
DGX Station Test Evidence
Not applicable.
Verification
Signed-off-by: Carlos Villela cvillela@nvidia.com