ci: finish executable workflow extraction - #7741
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>
📝 WalkthroughWalkthroughThe PR routes WSL provisioning and execution through a trusted PowerShell helper, updates WSL workflow security and trigger validation, and passes GitHub Actions ChangesEnvironment-based needs handling
Trusted WSL workflow execution
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant TrustedWslHelper
participant WslWorkspace
participant TestRunner
Workflow->>TrustedWslHelper: Provision WSL and resolve paths
TrustedWslHelper->>WslWorkspace: Install dependencies and sync source
Workflow->>TrustedWslHelper: Invoke build and test commands
TrustedWslHelper->>TestRunner: Run Vitest or E2E shards
sequenceDiagram
participant E2EWorkflow
participant GithubScript
participant ProcessEnv
E2EWorkflow->>GithubScript: Set NEEDS_JSON from toJSON(needs)
GithubScript->>ProcessEnv: Read process.env.NEEDS_JSON
ProcessEnv-->>GithubScript: Return JSON needs data
GithubScript->>GithubScript: Parse and use needs
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 |
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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/wsl-e2e.yaml (1)
113-123: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
npm ci --ignore-scriptshere to match the platform lane.
platform-vitest-main.yaml(Lines 243-246) usesnpm ci, while this lane usesnpm install, which can resolve versions outsidepackage-lock.jsonand make WSL E2E results non-reproducible. If the divergence is deliberate (e.g. this lane intentionally tolerates lock drift), ignore this.♻️ Align the install command
set -euo pipefail cd $workdir - npm install --ignore-scripts + npm ci --ignore-scripts npm run build:cli cd nemoclaw - npm install --ignore-scripts + npm ci --ignore-scripts npm run build🤖 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 @.github/workflows/wsl-e2e.yaml around lines 113 - 123, Update the dependency installation commands in the WSL script around Invoke-WslScript to use npm ci --ignore-scripts instead of npm install --ignore-scripts for both the root project and nemoclaw, preserving the existing build and directory flow.
🤖 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 `@tools/e2e/operations-workflow-boundary.mts`:
- Around line 51-53: Make the NEEDS_JSON validation semantic in
tools/e2e/operations-workflow-boundary.mts at lines 51-53 by verifying that
process.env.NEEDS_JSON is parsed and assigned to the actual needs value,
preferably through syntax-aware inspection rather than matching any raw
JSON.parse substring. In
test/e2e/support/e2e-operations-workflow-boundary.test.ts at lines 68-92, rename
the test with behavior-oriented wording and add independent cases covering
missing or malformed environment data, interpolation, and commented or unrelated
parsing so each rejection behavior is tested in isolation.
---
Nitpick comments:
In @.github/workflows/wsl-e2e.yaml:
- Around line 113-123: Update the dependency installation commands in the WSL
script around Invoke-WslScript to use npm ci --ignore-scripts instead of npm
install --ignore-scripts for both the root project and nemoclaw, preserving the
existing build and directory flow.
🪄 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: d00a613d-1e6d-4ffc-8eec-1edda03540c2
📒 Files selected for processing (9)
.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.tstools/e2e/operations-workflow-boundary.mts
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical dated changelog entry for NemoClaw v0.0.97 before the release plan captures `origin/main`. The entry groups the user-visible and maintainer-facing changes since v0.0.96 while preserving the Deferred dual-Station status, experimental runtime-identity boundary, and pending physical IGX validation. ## Changes - Add `docs/changelog/2026-07-28.mdx` with the parser-safe MDX SPDX comment and exact `## v0.0.97` heading. - Summarize the 43 merged PRs in the release range, omitting internal-only changes from the public entry and linking each grouped change to its most specific published documentation. - Keep the experimental Okta reference explicitly opt-in and outside normal onboarding, keep the two-Station path Deferred, and state that physical IGX Orin validation remains pending. ### Source summary - [#7440](#7440), [#7443](#7443), and [#7445](#7445) -> `docs/changelog/2026-07-28.mdx`: Document read-only host readiness reports and fail-closed platform qualification. - [#7030](#7030) -> `docs/changelog/2026-07-28.mdx`: Document the Deferred trusted two-Station vLLM evaluation. - [#7265](#7265) -> `docs/changelog/2026-07-28.mdx`: Document the bounded experimental direct-runner Okta runtime-identity reference. - [#7711](#7711) and [#7648](#7648) -> `docs/changelog/2026-07-28.mdx`: Document compatible-endpoint reasoning effort and retired NVIDIA Build model paths. - [#7746](#7746), [#7763](#7763), and [#7681](#7681) -> `docs/changelog/2026-07-28.mdx`: Document safe compatible-provider creation, replacement refusal, and narrow OpenShell bridge URL handling. - [#7641](#7641), [#7690](#7690), [#7631](#7631), and [#7710](#7710) -> `docs/changelog/2026-07-28.mdx`: Document paused-container recovery, recreation journaling, pre-mutation uninstall checks, and source-checkout OpenShell selection. - [#7624](#7624) and [#7762](#7762) -> `docs/changelog/2026-07-28.mdx`: Document Jetson release diagnostics and bounded render-device group propagation. - [#7639](#7639), [#7760](#7760), [#7721](#7721), and [#7761](#7761) -> `docs/changelog/2026-07-28.mdx`: Document Telegram, MCP media-type, Hermes image-mode, and locked-restart fixes. - [#7653](#7653) and [#7680](#7680) -> `docs/changelog/2026-07-28.mdx`: Document Deep Agents policy tasks and the bounded Claude Code OAuth path. - [#7679](#7679) -> `docs/changelog/2026-07-28.mdx`: Document the checksum-bound libssh2 and Python HTMLParser backports. - [#7655](#7655), [#7651](#7651), [#7664](#7664), [#7666](#7666), [#7670](#7670), [#7719](#7719), and [#7741](#7741) -> `docs/changelog/2026-07-28.mdx`: Document exact candidate E2E evidence, Launchable selection, diagnostic consolidation, and trusted WSL validation. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog contract, MDX header, heading uniqueness, and release-entry structure. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] 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: - [ ] 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: `docs-updated` - Evidence: The committed `docs/changelog/2026-07-28.mdx` blob exactly matches the reviewed file. Completeness, factual accuracy, link shape, parser-safe MDX header, one-sentence-per-line style, `.docs-skip` compliance, and bounded product claims passed. - Agent: Codex Desktop documentation writer subagent <!-- docs-review-head-sha: da6aa27 --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; this PR changes only the dated changelog. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## 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 — `npx vitest run test/changelog-docs.test.ts` passed 6/6. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not applicable to this doc-only release entry. - [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) — completed with 0 errors and 2 pre-existing Fern warnings. - [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) — native changelog entries use the required parser-safe MDX SPDX comment and intentionally have no frontmatter. --- Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added improved host readiness reporting and Jetson onboarding guidance. * Added controls for reasoning effort with compatible endpoints and enhanced managed MCP discovery. * Improved Deep Agents task publication and preset support. * **Bug Fixes** * Hardened provider switching, sandbox recovery, uninstall behavior, and Telegram connectivity. * Improved container image integrity checks, media-type handling, and checksum validation. * Enhanced vLLM evaluation behavior and release diagnostics. * **Documentation** * Added the NemoClaw v0.0.97 changelog. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
mainafter the helper landed.Related Issue
Fixes #6952
Fixes #6958
Changes
tools/wsl/ci-helper.ps1for WSL E2E and platform Vitest setup, script transfer, Node.js installation, and ext4 checkout synchronization. The workflow regression tests protect both consumers.needsthroughNEEDS_JSONinstead of interpolating GitHub expressions into executable source. The E2E operations boundary test rejects direct interpolation.needsreceives the parsed environment value. Isolated tests reject comments, unrelated assignments, and malformed wiring.Type of Change
Quality Gates
needsassignment and interpolation, and root separation.Documentation Writer Review
no-docs-neededb56897529e166385464c7e8ec051174f3af4f2aaagainst basec3ab0526eba41d1c681e92bbad3d174925aeaa6a. 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.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 unavailablenpx biome checkfor the changed files,npm run checks,npm run typecheck:cli, andgit diff --checkpassed.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