Skip to content

test(e2e): migrate gateway drift preflight to Vitest - #5139

Closed
cv wants to merge 18 commits into
mainfrom
codex/e2e-migrate-gateway-drift-preflight
Closed

test(e2e): migrate gateway drift preflight to Vitest#5139
cv wants to merge 18 commits into
mainfrom
codex/e2e-migrate-gateway-drift-preflight

Conversation

@cv

@cv cv commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Migrates the gateway drift preflight regression from its legacy shell wrapper into a Vitest live scenario. The regression workflow now runs the scenario directly through e2e-scenarios-live and uploads fixture artifacts from the shared E2E harness.

Related Issue

Refs #4941

Changes

  • Add test/e2e-scenario/live/gateway-drift-preflight.test.ts for gateway drift detection and preflight recovery coverage.
  • Replace test/e2e/test-gateway-drift-preflight.sh in regression-e2e.yaml with direct Vitest execution.
  • Extend test/regression-e2e-workflow.test.ts so the workflow contract points at the Vitest scenario and not the retired shell script.

Type of Change

  • 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)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

@cv cv self-assigned this Jun 10, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jun 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: cea19f41-3228-493b-b379-2115af9fc399

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/e2e-migrate-gateway-drift-preflight

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: gateway-drift-preflight-e2e
Optional E2E: None

Dispatch hint: gateway-drift-preflight-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/codex/e2e-simplify-migration-tracking
Head: HEAD
Confidence: high

Required E2E

  • gateway-drift-preflight-e2e (low; hermetic fake OpenShell/Docker regression job with 15 minute timeout): This is the directly modified existing E2E job. It must run to validate the workflow migration, root dependency install, Vitest live-scenario gate, fake OpenShell/Docker harness, fail-closed gateway drift assertions, and artifact upload path.

Optional E2E

  • None.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: .github/workflows/regression-e2e.yaml
  • jobs input: gateway-drift-preflight-e2e

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

E2E Scenario Advisor Recommendation

Required scenario E2E: e2e-scenarios-all
Optional scenario E2E: None

Dispatch required scenario E2E:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref>

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/codex/e2e-simplify-migration-tracking
Head: HEAD
Confidence: medium

Required scenario E2E

  • e2e-scenarios-all: A new live Vitest scenario test was added under test/e2e-scenario/live/. It is not a trusted-main live-supported typed registry scenario ID that can be targeted through the scenarios input, so run the scenario fan-out rather than inventing a targeted ID.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref>

Optional scenario E2E

  • None.

Relevant changed files

  • test/e2e-scenario/live/gateway-drift-preflight.test.ts

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 1 worth checking, 0 nice ideas
Since last review: 0 prior items resolved, 1 still applies, 0 new items found

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Limit inherited environment exposure in always-uploaded E2E artifacts (test/e2e-scenario/live/gateway-drift-preflight.test.ts:305): The migrated scenario still runs the CLI with `inheritEnv: true`, writes the combined command output into per-case artifacts, and the workflow uploads the full Vitest artifact directory with `if: always()`. The fixture redacts known secret patterns and this lane does not explicitly pass `NVIDIA_API_KEY` or `GITHUB_TOKEN`, so this is not a confirmed leak. However, inheriting the full runner environment gives repo-controlled child commands access to any ambient or future environment value, and values with unknown names or token shapes could be emitted and preserved in uploaded artifacts.
    • Recommendation: Prefer a minimal explicit environment for `runNemoclawCase` and the build step. If inheritance is required, keep this workflow lane free of secrets and pass explicit `redactionValues` for any additional inherited values that could appear in stdout/stderr.
    • Evidence: `runNemoclawCase` passes `inheritEnv: true` while overriding `HOME`, `PATH`, and `TMPDIR`, then writes `${result.stdout}${result.stderr}` to `command.out`; the workflow uploads `e2e-artifacts/vitest/gateway-drift-preflight/` on `always()`.

🌱 Nice ideas

  • None.
Consider writing more tests for
  • **Runtime validation** — Validate that `gateway-drift-preflight-e2e` invokes `e2e-scenarios-live` with `NEMOCLAW_RUN_E2E_SCENARIOS=1` and writes artifacts under `E2E_ARTIFACT_DIR`.. The PR changes a workflow/infrastructure path and a live E2E scenario for a sandbox-state trust boundary. The migrated Vitest scenario and workflow contract cover the code-level migration, but runtime validation of the changed workflow invocation remains valuable.
  • **Runtime validation** — Add or extend a workflow contract assertion that `gateway-drift-preflight-e2e.permissions.contents` remains exactly `read`.. The PR changes a workflow/infrastructure path and a live E2E scenario for a sandbox-state trust boundary. The migrated Vitest scenario and workflow contract cover the code-level migration, but runtime validation of the changed workflow invocation remains valuable.
  • **Acceptance clause:** Refs Adopt Vitest fixtures as the E2E scenario execution model #4941 — add test evidence or identify existing coverage. The PR body references Adopt Vitest fixtures as the E2E scenario execution model #4941, but deterministic linked-issue context did not include issue Adopt Vitest fixtures as the E2E scenario execution model #4941 clauses or comments, so literal issue acceptance criteria could not be mapped.
Since last review details

Current findings:

  • Limit inherited environment exposure in always-uploaded E2E artifacts (test/e2e-scenario/live/gateway-drift-preflight.test.ts:305): The migrated scenario still runs the CLI with `inheritEnv: true`, writes the combined command output into per-case artifacts, and the workflow uploads the full Vitest artifact directory with `if: always()`. The fixture redacts known secret patterns and this lane does not explicitly pass `NVIDIA_API_KEY` or `GITHUB_TOKEN`, so this is not a confirmed leak. However, inheriting the full runner environment gives repo-controlled child commands access to any ambient or future environment value, and values with unknown names or token shapes could be emitted and preserved in uploaded artifacts.
    • Recommendation: Prefer a minimal explicit environment for `runNemoclawCase` and the build step. If inheritance is required, keep this workflow lane free of secrets and pass explicit `redactionValues` for any additional inherited values that could appear in stdout/stderr.
    • Evidence: `runNemoclawCase` passes `inheritEnv: true` while overriding `HOME`, `PATH`, and `TMPDIR`, then writes `${result.stdout}${result.stderr}` to `command.out`; the workflow uploads `e2e-artifacts/vitest/gateway-drift-preflight/` on `always()`.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure refactor PR restructures code without intended behavior change labels Jun 10, 2026
@wscurran

Copy link
Copy Markdown
Contributor

Base automatically changed from codex/e2e-simplify-migration-tracking to main June 10, 2026 20:53
@jyaunches

Copy link
Copy Markdown
Contributor

Closing this draft codex migration PR as superseded by the simplified #5098 per-script migration process. The branch carries broad/stale framework-era diff noise; if this test is still needed, we'll salvage the useful core in a fresh/simple one-script PR using nemoclaw-e2e-legacy-migrate.

@jyaunches jyaunches closed this Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants