test(e2e): migrate test-tunnel-lifecycle.sh to vitest - #5587
Conversation
|
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 new live Vitest end-to-end test that installs NemoClaw into a sandbox, starts a Cloudflare quick tunnel, probes the public endpoint, and verifies tunnel stop. Wires this as a free-standing ChangesTunnel Lifecycle Live E2E Scenario
Sequence Diagram(s)(Skipped — sequence diagram already embedded in the hidden review stack artifact.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27956783433
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/e2e-vitest-scenarios.yaml (1)
2961-2961: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUse runner temp for Docker auth config instead of workspace.
DOCKER_CONFIGunder${{ github.workspace }}keeps registry auth material in the repo workspace. Prefer${{ runner.temp }}(as done in adjacent jobs) to reduce accidental credential exposure surface.Suggested diff
- DOCKER_CONFIG: ${{ github.workspace }}/.docker-config-tunnel-lifecycle + DOCKER_CONFIG: ${{ runner.temp }}/docker-config-tunnel-lifecycle🤖 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 at line 2961, The DOCKER_CONFIG environment variable is currently set to store the Docker auth config file in the repo workspace, which creates a security risk for accidental credential exposure. Change the DOCKER_CONFIG path from ${{ github.workspace }}/.docker-config-tunnel-lifecycle to ${{ runner.temp }}/.docker-config-tunnel-lifecycle to align with adjacent jobs and reduce the surface area for exposing sensitive registry authentication material.test/e2e-scenario/live/tunnel-lifecycle.test.ts (1)
407-424: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider explicit type narrowing instead of non-null assertions.
The
expect().toBeTruthy()at line 407 doesn't narrow the TypeScript type, requiring!assertions throughout. While logically safe (the loop always executes at least once), an explicit guard would be cleaner.♻️ Suggested improvement
expect(lastPublicProbe, "public tunnel probe should have run").toBeTruthy(); - if (lastPublicProbe!.httpCode !== "200") { + if (!lastPublicProbe) { + throw new Error("unreachable: public tunnel probe should have run"); + } + if (lastPublicProbe.httpCode !== "200") { if ( - isCloudflareTransientHttpCode(lastPublicProbe!.httpCode) || - isCloudflareTransientText(lastPublicProbe!.body) || + isCloudflareTransientHttpCode(lastPublicProbe.httpCode) || + isCloudflareTransientText(lastPublicProbe.body) || isCloudflareTransientText(readCloudflaredLog()) ) { skip( - `[Cloudflare fault] Tunnel URL never became reachable while local stayed healthy; last HTTP status ${lastPublicProbe!.httpCode}.`, + `[Cloudflare fault] Tunnel URL never became reachable while local stayed healthy; last HTTP status ${lastPublicProbe.httpCode}.`, ); } throw new Error( - `[NemoClaw fault] Tunnel returned unexpected HTTP ${lastPublicProbe!.httpCode} while local stayed healthy; body prefix: ${lastPublicProbe!.body.slice(0, 200)}`, + `[NemoClaw fault] Tunnel returned unexpected HTTP ${lastPublicProbe.httpCode} while local stayed healthy; body prefix: ${lastPublicProbe.body.slice(0, 200)}`, ); } - expect(lastPublicProbe!.body, "public tunnel must serve OpenClaw dashboard markers").toMatch( + expect(lastPublicProbe.body, "public tunnel must serve OpenClaw dashboard markers").toMatch( DASHBOARD_MARKER_PATTERN, );🤖 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/tunnel-lifecycle.test.ts` around lines 407 - 424, The code uses multiple non-null assertions on the `lastPublicProbe` variable (with the `!` operator), but the initial `expect(lastPublicProbe, "public tunnel probe should have run").toBeTruthy()` call doesn't narrow the TypeScript type. Add an explicit guard clause immediately after the expect statement (such as `if (!lastPublicProbe) throw new Error(...)`) to properly narrow the type for TypeScript, then remove all the trailing `!` assertions on `lastPublicProbe` references throughout the conditional blocks checking `lastPublicProbe!.httpCode`, `lastPublicProbe!.body`, etc.
🤖 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 @.github/workflows/e2e-vitest-scenarios.yaml:
- Line 2961: The DOCKER_CONFIG environment variable is currently set to store
the Docker auth config file in the repo workspace, which creates a security risk
for accidental credential exposure. Change the DOCKER_CONFIG path from ${{
github.workspace }}/.docker-config-tunnel-lifecycle to ${{ runner.temp
}}/.docker-config-tunnel-lifecycle to align with adjacent jobs and reduce the
surface area for exposing sensitive registry authentication material.
In `@test/e2e-scenario/live/tunnel-lifecycle.test.ts`:
- Around line 407-424: The code uses multiple non-null assertions on the
`lastPublicProbe` variable (with the `!` operator), but the initial
`expect(lastPublicProbe, "public tunnel probe should have run").toBeTruthy()`
call doesn't narrow the TypeScript type. Add an explicit guard clause
immediately after the expect statement (such as `if (!lastPublicProbe) throw new
Error(...)`) to properly narrow the type for TypeScript, then remove all the
trailing `!` assertions on `lastPublicProbe` references throughout the
conditional blocks checking `lastPublicProbe!.httpCode`,
`lastPublicProbe!.body`, etc.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5df7d70f-29ef-4889-b2a4-e6932035c106
📒 Files selected for processing (3)
.github/workflows/e2e-vitest-scenarios.yamltest/e2e-scenario/live/tunnel-lifecycle.test.tstest/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts
…el-lifecycle-vitest
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
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
|
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27958247974
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27958773719
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27959267892
|
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
Review findings by urgency: 0 required fixes, 1 item to resolve/justify, 0 in-scope improvements
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27960645101
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27961313365
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27962184227
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27962559753
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27963563847
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27964242839
|
…el-lifecycle-vitest # Conflicts: # .github/workflows/e2e-vitest-scenarios.yaml
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27966300871
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27967443514
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27967747511
|
…el-lifecycle-vitest # Conflicts: # test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27972261935
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27972545398
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27973755879
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27974607947
|
## Summary Restore issue #5800 parity package `P0-E` for merged bash-suite deltas only. ## Related Issues Refs #5800 Refs #5098 Refs #5197 Refs #5245 Refs #5508 Refs #5587 ## Scope gate - Package: `P0-E — Platform, tunnel, cloud-experimental, and docs parity decisions` - Included PRs all merged and touched `test/e2e`: yes — #5197, #5245, #5508, #5587 - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR #5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | E1 | #5197 | Deep Agents Code cloud-experimental Landlock check: `.deepagents` and `/tmp` writable, `/usr` and `/etc` read-only. | `hosted-compatible capable` | `ubuntu-repo-cloud-langchain-deepagents-code` live scenario now runs `05-deepagents-code-landlock-readonly.sh` after onboarding. | covered | | E2 | #5197 | Deep Agents Code arbitrary Python egress permits GitHub/PyPI and blocks Tavily/LangSmith/MCP/example.com absent explicit policy. | `hosted-compatible capable` | `ubuntu-repo-cloud-langchain-deepagents-code` live scenario now runs `06-deepagents-code-python-egress.sh` after onboarding. | covered | | E3 | #5245 | Trace timing signal is trusted summary only; raw target-controlled traces are not uploaded. | `none` | Existing `test/e2e-script-workflow.test.ts` and sanitizer tests cover trusted trace summary, scorecard comparison, and redaction boundary. | covered | | E4 | #5508 | GPU Local Ollama small-context config carries reduced OpenClaw compaction reserve. | `none` / local inference | `test/e2e-scenario/live/gpu-e2e.test.ts` now reads `/sandbox/.openclaw/openclaw.json` and asserts `reserveTokens` / `reserveTokensFloor` for small windows. | covered | | E5 | #5508 | Interactive OpenClaw TUI first turn must not fail auto-compaction. | `none` / local inference | No Vitest TUI fixture exists yet; config-level assertion covers the deterministic contract and the interactive smoke remains platform/manual-runner follow-up. Owner: NemoClaw maintainers; date: 2026-06-25; tracked in #5800 closeout. | waived/follow-up | | E6 | #5587 | Cloudflared exact-version override validates Debian version syntax before APT install. | `hosted-compatible capable` for live tunnel job; resolver itself `none` | Existing `test/cloudflared-version-resolver.test.ts`; tunnel workflow boundary already requires resolver use before `apt-get install cloudflared=${cf_version}`. | covered | ## Inference mode support - Default mode for touched live targets: `hosted-compatible capable` for Deep Agents Code and tunnel lifecycle; local Ollama/GPU uses local inference; trace/resolver tests are `none`. - Real inference support preserved: yes for Deep Agents Code scenario via `NVIDIA_INFERENCE_API_KEY`; yes for tunnel lifecycle via existing hosted-compatible route; local Ollama GPU remains local inference. - Modes validated in this PR: local unit/support tests plus live scenario selection without secret; selective hosted/GPU workflow dispatch required after PR opens. - If not validated with real inference: local machine lacks `NVIDIA_INFERENCE_API_KEY`, GPU runner, and Docker daemon; selective GitHub Actions will validate runner/secret boundaries. ## Validation - [x] `npm run build:cli` - [x] `npx vitest run test/e2e-scenario/support-tests/e2e-scenario-matrix.test.ts test/e2e-scenario/support-tests/e2e-scenario-registry.test.ts test/langchain-deepagents-code-image.test.ts test/ollama-local-openclaw-config-propagation.test.ts test/cloudflared-version-resolver.test.ts --silent=false --reporter=default` - [x] `NEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/registry-scenarios.test.ts -t '^ubuntu-repo-cloud-langchain-deepagents-code$' --silent=false --reporter=verbose` (local result: skipped due missing `NVIDIA_INFERENCE_API_KEY`, confirms scenario wiring) - [ ] hosted/public selective E2E workflow, if required by classification: pending PR dispatch ## Follow-ups / waivers - E5: Interactive TUI first-turn smoke remains waived until a Vitest TUI fixture exists. Owner: NemoClaw maintainers; date: 2026-06-25; deterministic config contract is covered here. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a new cloud onboarding path for LangChain Deep Agents Code, including dedicated fixtures and live scenario coverage. * Surfaced new “cloud experimental” checks in live run planning and scenario matrix output. * **Bug Fixes** * Improved GPU live E2E validation by asserting the reserve-token compaction policy. * Updated Deep Agents Code expected gateway health handling to be optional. * **Security/Policy** * Expanded Deep Agents Code Python egress allowlists for approved GitHub/PyPI hosts. * **Tests** * Updated E2E checks and assertions (including Python egress probe behavior) and adjusted scenario/support-matrix expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Carlos Villela <cvillela@nvidia.com>
## Summary Restore issue NVIDIA#5800 parity package `P0-E` for merged bash-suite deltas only. ## Related Issues Refs NVIDIA#5800 Refs NVIDIA#5098 Refs NVIDIA#5197 Refs NVIDIA#5245 Refs NVIDIA#5508 Refs NVIDIA#5587 ## Scope gate - Package: `P0-E — Platform, tunnel, cloud-experimental, and docs parity decisions` - Included PRs all merged and touched `test/e2e`: yes — NVIDIA#5197, NVIDIA#5245, NVIDIA#5508, NVIDIA#5587 - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR NVIDIA#5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | E1 | NVIDIA#5197 | Deep Agents Code cloud-experimental Landlock check: `.deepagents` and `/tmp` writable, `/usr` and `/etc` read-only. | `hosted-compatible capable` | `ubuntu-repo-cloud-langchain-deepagents-code` live scenario now runs `05-deepagents-code-landlock-readonly.sh` after onboarding. | covered | | E2 | NVIDIA#5197 | Deep Agents Code arbitrary Python egress permits GitHub/PyPI and blocks Tavily/LangSmith/MCP/example.com absent explicit policy. | `hosted-compatible capable` | `ubuntu-repo-cloud-langchain-deepagents-code` live scenario now runs `06-deepagents-code-python-egress.sh` after onboarding. | covered | | E3 | NVIDIA#5245 | Trace timing signal is trusted summary only; raw target-controlled traces are not uploaded. | `none` | Existing `test/e2e-script-workflow.test.ts` and sanitizer tests cover trusted trace summary, scorecard comparison, and redaction boundary. | covered | | E4 | NVIDIA#5508 | GPU Local Ollama small-context config carries reduced OpenClaw compaction reserve. | `none` / local inference | `test/e2e-scenario/live/gpu-e2e.test.ts` now reads `/sandbox/.openclaw/openclaw.json` and asserts `reserveTokens` / `reserveTokensFloor` for small windows. | covered | | E5 | NVIDIA#5508 | Interactive OpenClaw TUI first turn must not fail auto-compaction. | `none` / local inference | No Vitest TUI fixture exists yet; config-level assertion covers the deterministic contract and the interactive smoke remains platform/manual-runner follow-up. Owner: NemoClaw maintainers; date: 2026-06-25; tracked in NVIDIA#5800 closeout. | waived/follow-up | | E6 | NVIDIA#5587 | Cloudflared exact-version override validates Debian version syntax before APT install. | `hosted-compatible capable` for live tunnel job; resolver itself `none` | Existing `test/cloudflared-version-resolver.test.ts`; tunnel workflow boundary already requires resolver use before `apt-get install cloudflared=${cf_version}`. | covered | ## Inference mode support - Default mode for touched live targets: `hosted-compatible capable` for Deep Agents Code and tunnel lifecycle; local Ollama/GPU uses local inference; trace/resolver tests are `none`. - Real inference support preserved: yes for Deep Agents Code scenario via `NVIDIA_INFERENCE_API_KEY`; yes for tunnel lifecycle via existing hosted-compatible route; local Ollama GPU remains local inference. - Modes validated in this PR: local unit/support tests plus live scenario selection without secret; selective hosted/GPU workflow dispatch required after PR opens. - If not validated with real inference: local machine lacks `NVIDIA_INFERENCE_API_KEY`, GPU runner, and Docker daemon; selective GitHub Actions will validate runner/secret boundaries. ## Validation - [x] `npm run build:cli` - [x] `npx vitest run test/e2e-scenario/support-tests/e2e-scenario-matrix.test.ts test/e2e-scenario/support-tests/e2e-scenario-registry.test.ts test/langchain-deepagents-code-image.test.ts test/ollama-local-openclaw-config-propagation.test.ts test/cloudflared-version-resolver.test.ts --silent=false --reporter=default` - [x] `NEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/registry-scenarios.test.ts -t '^ubuntu-repo-cloud-langchain-deepagents-code$' --silent=false --reporter=verbose` (local result: skipped due missing `NVIDIA_INFERENCE_API_KEY`, confirms scenario wiring) - [ ] hosted/public selective E2E workflow, if required by classification: pending PR dispatch ## Follow-ups / waivers - E5: Interactive TUI first-turn smoke remains waived until a Vitest TUI fixture exists. Owner: NemoClaw maintainers; date: 2026-06-25; deterministic config contract is covered here. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a new cloud onboarding path for LangChain Deep Agents Code, including dedicated fixtures and live scenario coverage. * Surfaced new “cloud experimental” checks in live run planning and scenario matrix output. * **Bug Fixes** * Improved GPU live E2E validation by asserting the reserve-token compaction policy. * Updated Deep Agents Code expected gateway health handling to be optional. * **Security/Policy** * Expanded Deep Agents Code Python egress allowlists for approved GitHub/PyPI hosts. * **Tests** * Updated E2E checks and assertions (including Python egress probe behavior) and adjusted scenario/support-matrix expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Summary
Migrate
test/e2e/test-tunnel-lifecycle.shwith the simplest equivalent live Vitest coverage.Related Issues
Refs #5098
Contract mapping
nemoclaw tunnel startand surfaces a trycloudflare URL in status.test/e2e-scenario/live/tunnel-lifecycle.test.tsstarts the real tunnel and pollsnemoclaw statusfor a trycloudflare URL.nemoclawCLI, real hostcloudflaredquick tunnel, andcloudflared.logdiagnosis.nemoclaw tunnel stopand pollsnemoclaw statusfor URL removal.Simplicity check
nightly-e2e.yamljobtunnel-lifecycle-e2eviae2e-script.yaml,runs-on: ubuntu-latest, timeout 60, Docker/OpenShell, sudo-capable cloudflared install,NVIDIA_INFERENCE_API_KEYe2e-vitest-scenarios.yamljobtunnel-lifecycle-vitestonubuntu-latesttunnel-lifecycle-vitest; legacy shell deletion/workflow retirement deferred to Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 Phase 11e2e-vitest-scenarios.yamlwithjobs=tunnel-lifecycle-vitestVerification
npm ci --ignore-scriptsnpm run build:clinpx tsc --noEmit --allowImportingTsExtensions --moduleResolution bundler --module esnext --target es2022 --types node,vitest test/e2e-scenario/live/tunnel-lifecycle.test.tsNEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/tunnel-lifecycle.test.ts --silent=false --reporter=default(local no-secret path skips)npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts --silent=false --reporter=defaultnpm run typecheck:cligit diff --checkSummary by CodeRabbit
PR Review Advisor response
PRA-1/PRA-2(redirect boundary): removedcurl -Lfrom the public tunnel probe. The source boundary is the initial*.trycloudflare.comURL fromnemoclaw status; redirects are not part of the migrated legacy contract, so a 3xx response is treated as probe output rather than followed to an off-allowlist host. Removal condition: only add explicit redirect target inspection if NemoClaw later documents same-host quick-tunnel redirects.PRA-T1runtime validation: same-runner livetunnel-lifecycle-vitestpassed on current head878e93a1in run27967747511, covering real install/onboard, Docker/OpenShell, hostcloudflared, trycloudflare URL surfacing innemoclaw status, public dashboard probe without redirect-following, tunnel stop, and status cleanup.PRA-T2/PRA-T3Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 acceptance mapping: trusted issue Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 defines conversion as equivalent typed Vitest coverage, preserving shell/system boundaries, deterministic artifacts/cleanup/timeouts/redaction, PR contract mapping, same-runner workflow wiring, and explicit Phase 11 deferral for shell/workflow retirement. This PR maps that scope as:test/e2e-scenario/live/tunnel-lifecycle.test.ts+test/e2e-scenario/live/tunnel-lifecycle-helpers.tsreplacetest/e2e/test-tunnel-lifecycle.sh;cloudflared, trycloudflare probe, stop/status cleanup;tunnel-lifecycle-vitestin.github/workflows/e2e-vitest-scenarios.yamlruns onubuntu-latestand passes in run27967747511;PRA-T2/PRA-T3support coverage: added focused support tests provingclassifyCloudflaredLog()returnsnemoclaw_localfor localhost/origin-refused logs andcloudflarefor representative quick-tunnel transient registration failures.PRA-1: moved tunnel-lifecycle Docker auth out of job-levelDOCKER_CONFIGand into the existing${RUNNER_TEMP}setup-step pattern, with validator coverage rejecting job-level Docker auth and${{ github.workspace }}drift.PRA-1/PRA-3: removed the newest-log fallback. The source boundary is now the exact per-sandbox log/tmp/nemoclaw-services-${NEMOCLAW_SANDBOX_NAME}/cloudflared.log; if it is absent the contract reportsnemoclaw_no_spawninstead of attributing another run's log.validateTunnelLifecycleVitestJob()and support tests for checkout credential persistence,npm ci --ignore-scripts, artifact upload safety, Docker auth cleanup/location, unrelatedcloudflared.logattribution, and no-redirect public probe construction.