test(e2e): migrate network policy to Vitest [ANCHOR-6]#5226
Merged
Conversation
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:
📝 WalkthroughWalkthroughAdds a network-policy live Vitest E2E scenario and integrates it into CI: splits free-standing vs registry-driven scenario selection when generating the test matrix, gates the registry-driven job on a non-empty matrix, introduces the ChangesNetwork-Policy Live E2E Test Suite
Sequence Diagram(s)sequenceDiagram
participant User as inputs.scenarios / inputs.jobs
participant Validate as validate-jobs
participant Matrix as generate-matrix
participant Runner as live-scenarios (registry-driven)
participant FreeJob as network-policy-vitest
participant Report as report-to-pr
User->>Validate: submit selectors
Validate->>Matrix: validate & split scenarios
Matrix->>Runner: registry-driven matrix (if not '[]')
User->>FreeJob: request free-standing job (network-policy-vitest)
FreeJob->>Report: artifacts & completion
Runner->>Report: registry-driven completion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Contributor
PR Review AdvisorFindings: 0 needs attention, 3 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
![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: 2
🧹 Nitpick comments (1)
test/e2e-scenario/live/network-policy.test.ts (1)
348-727: 🏗️ Heavy liftSplit the main scenario into smaller local contract helpers.
This single callback now mixes onboarding, preset application, package-manager probes, SSRF checks, host-gateway validation, and permissive-mode assertions. That makes failures harder to localize and pushes well past the repo's low-complexity JS/TS guideline. Extract a few file-local helpers or subtests for the major contract blocks instead of keeping the full scenario in one function. As per coding guidelines, "Keep function complexity low in JavaScript and TypeScript code."
🤖 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-scenario/live/network-policy.test.ts` around lines 348 - 727, The test callback passed to RUN_NETWORK_POLICY_TEST is too large and mixes onboarding, preset application, package-manager probes, SSRF/host-gateway checks and permissive-mode assertions; split it into smaller, focused helpers or subtests (e.g., extract onboarding logic around runNemoclaw(... "onboard"...) and subsequent denyDefault check into an onboardAndDenyDefault helper, move preset operations using applyPreset/runNemoclaw/connectProbe/brewProbe into presetHelpers, isolate SSRF and inference checks using fetchStatus/isPrivateIp/inference into ssrfAndInferenceHelpers, and isolate host-gateway logic around startMarkerServer/writeHostGatewayPolicy/webFetch into hostGatewayHelper). Replace the big inline callback body with sequential calls to these file-local async functions (or Vitest subtests) to keep function complexity low, maintain existing assertions (using SANDBOX_NAME, applyPreset, sandboxBash, fetchStatus, startMarkerServer, writeHostGatewayPolicy, runNemoclaw, artifacts) and preserve cleanup handling and timeouts.Source: Coding guidelines
🤖 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/e2e-vitest-scenarios.yaml:
- Around line 280-341: The network-policy-vitest job fails because the test
(invocation of openshell at line 387 in
test/e2e-scenario/live/network-policy.test.ts) requires OpenShell to be
installed; add the same OpenShell bootstrap/install step used by other
sandbox-backed live jobs into this job before the "Run network-policy live test"
step (i.e., add an install/bootstrap step that provisions the openshell binary
or runtime so that the test's call to openshell can spawn successfully).
In `@test/e2e-scenario/live/network-policy.test.ts`:
- Around line 508-513: Replace the loose status checks around the Slack preset
with origin-specific assertions: assert that slackBefore (from fetchStatus)
equals/matches the blocked status (e.g., /STATUS_403/) rather than just "not
2xx/3xx", and assert that slackAfter equals/matches a true success status (e.g.,
/STATUS_200/) rather than allowing any 2xx-4xx; update the assertions around
slackBefore, slackApply, and slackAfter (and keep using fetchStatus and
applyPresetInteractively) so the test proves the preset changed reachability by
checking a specific blocked code before and a specific success code after.
---
Nitpick comments:
In `@test/e2e-scenario/live/network-policy.test.ts`:
- Around line 348-727: The test callback passed to RUN_NETWORK_POLICY_TEST is
too large and mixes onboarding, preset application, package-manager probes,
SSRF/host-gateway checks and permissive-mode assertions; split it into smaller,
focused helpers or subtests (e.g., extract onboarding logic around
runNemoclaw(... "onboard"...) and subsequent denyDefault check into an
onboardAndDenyDefault helper, move preset operations using
applyPreset/runNemoclaw/connectProbe/brewProbe into presetHelpers, isolate SSRF
and inference checks using fetchStatus/isPrivateIp/inference into
ssrfAndInferenceHelpers, and isolate host-gateway logic around
startMarkerServer/writeHostGatewayPolicy/webFetch into hostGatewayHelper).
Replace the big inline callback body with sequential calls to these file-local
async functions (or Vitest subtests) to keep function complexity low, maintain
existing assertions (using SANDBOX_NAME, applyPreset, sandboxBash, fetchStatus,
startMarkerServer, writeHostGatewayPolicy, runNemoclaw, artifacts) and preserve
cleanup handling and timeouts.
🪄 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: ec0be148-7809-4ed2-91c2-c1ed41561074
📒 Files selected for processing (2)
.github/workflows/e2e-vitest-scenarios.yamltest/e2e-scenario/live/network-policy.test.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/e2e-vitest-scenarios.yaml (1)
333-343:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInstall OpenShell before running the network-policy live test.
Line 341 executes a test that checks
openshell --versionbefore onboarding, but this job still doesn’t provision OpenShell. That can keep failing withspawn openshell ENOENT. Add the same OpenShell bootstrap/install step used by other sandbox-backed live jobs before this run step.🤖 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/e2e-vitest-scenarios.yaml around lines 333 - 343, The network-policy live test job runs tests that call `openshell --version` but never provisions OpenShell, causing `spawn openshell ENOENT`; add the same OpenShell bootstrap/install step used in the other sandbox-backed live jobs before the `npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/network-policy.test.ts` run step so `openshell` is available (mirror the install/bootstrap commands used in the other live jobs to ensure `openshell --version` succeeds).Source: Pipeline failures
🧹 Nitpick comments (1)
tools/e2e-scenarios/workflow-boundary.mts (1)
487-488: ⚡ Quick winExtend boundary checks to all free-standing selective-filter jobs.
The boundary validator now enforces selective
job.ifonly for two jobs. It still doesn’t assert the selective filters fornetwork-policy-vitestandopenclaw-tui-chat-correlation-vitest(workflow Lines 283 and 359), so regressions there won’t be caught by this support layer.🤖 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/e2e-scenarios/workflow-boundary.mts` around lines 487 - 488, The boundary checks only call validateOpenShellVersionPinVitestJob and validateOnboardNegativePathsVitestJob; add validation for the remaining selective-filter jobs so they enforce job.if as well. Update the invocation in workflow-boundary.mts to also call validators for "network-policy-vitest" and "openclaw-tui-chat-correlation-vitest" (either by adding new functions like validateNetworkPolicyVitestJob and validateOpenclawTuiChatCorrelationVitestJob or by extending an existing validator to accept a list of job names), and ensure those validators assert the selective job.if condition the same way as validateOpenShellVersionPinVitestJob/validateOnboardNegativePathsVitestJob do.
🤖 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.
Duplicate comments:
In @.github/workflows/e2e-vitest-scenarios.yaml:
- Around line 333-343: The network-policy live test job runs tests that call
`openshell --version` but never provisions OpenShell, causing `spawn openshell
ENOENT`; add the same OpenShell bootstrap/install step used in the other
sandbox-backed live jobs before the `npx vitest run --project e2e-scenarios-live
test/e2e-scenario/live/network-policy.test.ts` run step so `openshell` is
available (mirror the install/bootstrap commands used in the other live jobs to
ensure `openshell --version` succeeds).
---
Nitpick comments:
In `@tools/e2e-scenarios/workflow-boundary.mts`:
- Around line 487-488: The boundary checks only call
validateOpenShellVersionPinVitestJob and validateOnboardNegativePathsVitestJob;
add validation for the remaining selective-filter jobs so they enforce job.if as
well. Update the invocation in workflow-boundary.mts to also call validators for
"network-policy-vitest" and "openclaw-tui-chat-correlation-vitest" (either by
adding new functions like validateNetworkPolicyVitestJob and
validateOpenclawTuiChatCorrelationVitestJob or by extending an existing
validator to accept a list of job names), and ensure those validators assert the
selective job.if condition the same way as
validateOpenShellVersionPinVitestJob/validateOnboardNegativePathsVitestJob do.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: aae9ad9d-48b7-48d2-b0e2-77b92052a021
📒 Files selected for processing (3)
.github/workflows/e2e-vitest-scenarios.yamltest/e2e-scenario/support-tests/e2e-scenarios-workflow.test.tstools/e2e-scenarios/workflow-boundary.mts
jyaunches
changed the title
jyaunches
changed the title
jyaunches
changed the title
Contributor
Author
|
Maintainer simplicity/security review for #5098 — request changes. This can stay as the network-policy anchor PR, but please make the migrated Vitest coverage simpler and less false-positive-prone. Required:
Goal: preserve the security/policy anchor in this PR, but make the test read as explicit contract phases with narrow denial evidence. |
# Conflicts: # .github/workflows/e2e-vitest-scenarios.yaml # test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts # tools/e2e-scenarios/workflow-boundary.mts
…ork-policy-simple
…ork-policy-simple
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e-vitest-scenarios.yaml (1)
92-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFree-standing scenario ids are filtered out before any job can run them.
With
inputs.scenarios=network-policy, Lines 109-120 removenetwork-policyfromregistry_scenariosand forcematrix="[]", but Lines 243, 284, 329, and 412 only dispatch the matching free-standing jobs frominputs.jobs. The result is thatlive-scenariosskips and every free-standing job also skips, so the documentedscenarios=network-policypath runs nothing.Minimal fix
- if: ${{ (inputs.jobs == '' && inputs.scenarios == '') || contains(format(',{0},', inputs.jobs), ',network-policy-vitest,') }} + if: ${{ (inputs.jobs == '' && inputs.scenarios == '') || contains(format(',{0},', inputs.jobs), ',network-policy-vitest,') || contains(format(',{0},', inputs.scenarios), ',network-policy,') }}Apply the same pattern to the other scenario-backed free-standing jobs (
openshell-version-pin,onboard-negative-paths,openclaw-tui-chat-correlation), or emit the filtered free-standing ids fromgenerate-matrixand route jobs from that output.Also applies to: 243-243, 284-284, 329-329, 412-412
🤖 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/e2e-vitest-scenarios.yaml around lines 92 - 123, The current generate-matrix logic (free_standing_scenarios array + is_free_standing_scenario function) strips free-standing ids out of registry_scenarios and sets matrix="[]" when only free-standing scenarios are requested, causing downstream job dispatch (jobs matching openshell-version-pin, onboard-negative-paths, openclaw-tui-chat-correlation, network-policy) to skip; fix by emitting the selected free-standing ids from this script (e.g., set an output like free_standing_selected CSV or include them in matrix when registry_scenarios is empty) instead of forcing matrix="[]", and update the job dispatchers that currently read matrix to consume that output (use the same free_standing_scenarios/is_free_standing_scenario filtering to populate the output), ensuring registry_scenarios, args and matrix logic remain consistent.
🤖 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.
Outside diff comments:
In @.github/workflows/e2e-vitest-scenarios.yaml:
- Around line 92-123: The current generate-matrix logic (free_standing_scenarios
array + is_free_standing_scenario function) strips free-standing ids out of
registry_scenarios and sets matrix="[]" when only free-standing scenarios are
requested, causing downstream job dispatch (jobs matching openshell-version-pin,
onboard-negative-paths, openclaw-tui-chat-correlation, network-policy) to skip;
fix by emitting the selected free-standing ids from this script (e.g., set an
output like free_standing_selected CSV or include them in matrix when
registry_scenarios is empty) instead of forcing matrix="[]", and update the job
dispatchers that currently read matrix to consume that output (use the same
free_standing_scenarios/is_free_standing_scenario filtering to populate the
output), ensuring registry_scenarios, args and matrix logic remain consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8c555430-0fa9-447f-9719-0a2b44649367
📒 Files selected for processing (2)
.github/workflows/e2e-vitest-scenarios.yamltools/e2e-scenarios/workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/e2e-scenarios/workflow-boundary.mts
…ork-policy-simple
…ork-policy-simple
…ork-policy-simple
…ork-policy-simple
…ork-policy-simple
…ork-policy-simple
Contributor
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27372501486
|
Contributor
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27373045359
|
cv
approved these changes
Jun 11, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Approved. Normal PR checks are green on the latest head, review feedback is addressed, unresolved threads are clear, and network-policy-vitest passed. The remaining full scenario fan-out failures appear tied to unstable third-party NVIDIA inference endpoint validation rather than this PR.
Contributor
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27373045359
|
1 similar comment
Contributor
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27373045359
|
…ork-policy-simple # Conflicts: # .github/workflows/e2e-vitest-scenarios.yaml # test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts # tools/e2e-scenarios/workflow-boundary.mts
79 tasks
This was referenced Jun 12, 2026
wscurran
added
area: ci
This was referenced Jun 12, 2026
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
Migrate
test/e2e/test-network-policy.shinto focused live Vitest coverage for the security/policy anchor in #5098.Related Issue
Refs #5098
Refs #4014
Changes
test/e2e-scenario/live/network-policy.test.tsas a free-standing live Vitest test for the legacy network-policy contracts.policy-add, dry-run no-op behavior, per-binary Jira policy enforcement,/prochot-reload, inference.local exemption, SSRF private-IP checks, host-gatewayweb_fetchallow/deny, and permissive mode.network-policy-vitestjob toe2e-vitest-scenarios.yamlso the new test can run viaworkflow_dispatchwithscenarios=network-policywithout invoking the registry scenario matrix.Contract mapping
example.com.network-policy.test.tsprobes from inside the sandbox with Node fetch.network-policy.test.tsappliesbrew,pypi,slack,jira,npmand asserts sandbox curl/Node behavior.nemoclaw <sandbox> policy-add,openshell policy update, sandbox egress./proc/1/statstart time before/after policy change./procprobe.web_fetchcan reach approvedhost.openshell.internaltarget and cannot reach unapproved host-gateway ports.web_fetchruntime.openshell policy setand assertsnpm pingsucceeds.Simplicity check
Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Focused validation run locally:
npm run build:clinpm run typecheck:clinpx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts --silent=false --reporter=defaultNEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/network-policy.test.ts --silent=false --reporter=default(skips withoutNVIDIA_API_KEY)npm run test-size:checkgit diff --checkSigned-off-by: Julie Yaunches jyaunches@nvidia.com
Summary by CodeRabbit
Tests
Chores