refactor(e2e): centralize and validate the common live-Vitest invocation

[souvikDevloper]

Refs #6961 (parent epic #6952).

This is the focused first slice the issue invites ("adopt only in jobs that share the exact current contract; report which jobs were left explicit and why"). It delivers the testable, security-relevant core — a validated live-Vitest invocation helper — and adopts it in the one job that shares the pure common contract.

What this does

e2e.yaml repeats the npx vitest run --project e2e-live … --reporter=default --reporter=test/e2e/risk-signal-reporter.ts shape across many jobs.

tools/e2e/live-vitest-invocation.mts owns that shape. A job supplies only a test path and -t selector; the project, reporters, and silence flag are fixed. Inputs cross a trust boundary (matrix-derived), so they are validated before any command is built:

Input	Rejected
project	anything but e2e-live
test path	outside test/e2e/live/, .. traversal, absolute, shell metacharacters, non-.test.ts
selector	shell metacharacters in the expanded value

Rejection is fail-closed — nothing is quoted-away and run. The built argv is returned as an array (never a shell string).

Adopted in the canonical live (registry-targets) job. The NVIDIA_INFERENCE_API_KEY / TARGET_ID env flow is unchanged; the workflow-boundary validator for that job now asserts the helper invocation and inputs instead of the raw command.

Tests

test/e2e/support/live-vitest-invocation.test.ts (14) covers the project, path-traversal, absolute-path, shell-metacharacter, .test.ts, and selector rejections, plus argv building and fail-closed behavior on invalid input. typecheck:cli and biome are clean; I verified no new validateE2eWorkflowBoundary errors and no new test failures against a clean main baseline (tools/**/*.mts is covered by tsconfig.cli.json).

Intentionally left explicit (and why)

Per the acceptance criteria, here's what this slice does not change:

• The other ~70 live-Vitest sites don't share the exact contract. Most prepend export PATH=… and OPENSHELL_BIN=… setup, or run different files (skill-agent.test.ts, etc.). Adopting them needs helper inputs this slice deliberately omits; each also has its own workflow-boundary assertions to migrate. Better as follow-up increments than a 70-site sweep on the most critical CI workflow.
• Docker Hub auth setup stays inline. It is a single YAML anchor (&dockerhub-auth, aliased ~69×) that lives on the trusted workflow revision. Moving the secret-bearing login into a .github/scripts/ file would make it load from the tested checkout — branch-controllable — which the issue's own boundary explicitly forbids ("authentication … execute only from an immutable trusted action revision, never from the tested checkout"). The paired docker-auth-cleanup.sh is safe to keep as a script because it handles no secrets and validates paths. A trusted setup extraction would need an immutable-action mechanism; flagging for design rather than weakening the boundary here.
• The four expect host installs — the issue itself says keep inline if the immutable-action rollout would weaken auditability; unchanged.

Coordination

Behavior-preserving and touches only the one job's invocation body, so it composes with #6145's trigger changes rather than conflicting.

Signed-off-by: Souvik Ghosh 138186578+souvikDevloper@users.noreply.github.com

Summary by CodeRabbit

• Improvements

  • Live end-to-end test runs now use a standardized invocation that validates test paths, projects, and selectors before execution.
  • Test execution continues to use the expected reporting configuration and provides clearer safeguards for invalid inputs.

• Tests

  • Added coverage for valid and invalid live test parameters, argument construction, and fail-closed behavior.
  • Updated workflow validation to recognize the new live test invocation format.

[copy-pr-bot]

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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1b8efe37-f84f-4990-b001-0bcde0391860

📥 Commits

Reviewing files that changed from the base of the PR and between d03e9d7 and 587a7fb.

📒 Files selected for processing (4)

• .github/workflows/e2e.yaml
• test/e2e/support/live-vitest-invocation.test.ts
• tools/e2e/live-vitest-invocation.mts
• tools/e2e/workflow-boundary.mts

---

📝 Walkthrough

Walkthrough

Adds a validated live Vitest invocation helper, tests its input and argument handling, and updates the E2E workflow and boundary validator to use the helper.

Changes

Live Vitest invocation

Layer / File(s)	Summary
Invocation validation and execution tools/e2e/live-vitest-invocation.mts	Defines live Vitest constants, validates project, test path, and selector inputs, builds fixed Vitest arguments, and spawns the command without a shell.
Invocation validation tests test/e2e/support/live-vitest-invocation.test.ts	Tests accepted and rejected inputs, exact argument construction, and fail-closed behavior.
Workflow integration and boundary checks .github/workflows/e2e.yaml, tools/e2e/workflow-boundary.mts	Replaces the direct Vitest command with the helper invocation and updates required command-fragment validation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant E2EWorkflow
  participant LiveVitestInvocation
  participant Npx
  participant Vitest
  E2EWorkflow->>LiveVitestInvocation: pass test path and target selector
  LiveVitestInvocation->>LiveVitestInvocation: validate inputs and build argv
  LiveVitestInvocation->>Npx: spawn Vitest without shell
  Npx->>Vitest: run live E2E test with configured reporters

Loading

Possibly related issues

• Consolidate repeated live E2E invocation and Docker-auth setup into pinned helper actions #6961 — The PR implements the trusted live-Vitest helper and updates the workflow to use it.

Suggested labels: area: e2e, refactor

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: centralizing and validating the shared live-Vitest invocation in E2E tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

PR Review Advisor — Informational

Advisor assessment: Informational / medium confidence
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions
Status: Canonical ledger: 0 blocker(s), 1 warning(s), 0 suggestion(s).

Model lanes

• GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 1 warning · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 2 warnings · 0 suggestions
• Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 1 more warning, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-onboard, credential-sanitization, security-posture

1 optional E2E recommendation

• ubuntu-repo-cloud-langchain-deepagents-code

1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — Exercise the new helper-invocation workflow contract with mutations

• Location: tools/e2e/workflow-boundary.mts:3992
• Category: tests
• Problem: The validator now requires the live workflow to invoke `tools/e2e/live-vitest-invocation.mts run` with the fixed `--test-path` and `--selector`, but no checked-in workflow-boundary test mutates those new fragments and asserts that validation rejects their removal or drift. The helper unit tests do not cover this workflow-to-helper caller contract.
• Impact: A future workflow edit could omit or alter the helper subcommand, path, or selector and thereby change live test selection or bypass the intended canonical invocation without a focused regression proving the boundary validator catches it.
• Recommendation: Add focused mutations of the live `Run live E2E tests` step that independently remove/change the helper `run` subcommand, `--test-path`, and `--selector`, and assert the corresponding `validateE2eWorkflowBoundary` errors.
• Verification: Inspect `test/e2e/support/e2e-workflow-trace.test.ts` and other workflow-boundary tests for mutations of the new helper invocation fragments; none currently assert those validator failures.
• Test coverage: A workflow-boundary test that mutates each of the helper command, fixed registry test path, and anchored TARGET_ID selector, then expects the specific validation error for each drift.
• Evidence: `tools/e2e/workflow-boundary.mts` newly validates the helper `run` subcommand and fixed `--test-path`/`--selector` fragments. `test/e2e/support/live-vitest-invocation.test.ts` tests helper input validation and CLI behavior, not mutations of `.github/workflows/e2e.yaml` against these new validator checks. The inspected workflow-boundary mutation coverage tests trace sanitizer ordering, while the static inventory found no test for the new invocation-fragment checks.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[cv]

Thanks for taking on this consolidation. The pure argument validation is a useful foundation, but the current CLI can silently skip E2E, so I don't think this is safe to merge yet.

• Blocking: unsupported or misspelled subcommands exit successfully without running tests. The direct-entry guard calls runCli() only when process.argv[2] === "run"; every other value falls through and exits 0. I reproduced node --experimental-strip-types tools/e2e/live-vitest-invocation.mts runx returning 0. The workflow boundary check also uses a substring match for ...mts run, so runx satisfies it. Please make missing/unsupported subcommands fail and add direct CLI plus boundary regression tests.

• Termination behavior is not preserved. When the child is terminated by a signal or spawning fails, spawnSync has no numeric status and the wrapper converts that to exit 1. The linked issue requires exit behavior to remain unchanged; please preserve or deliberately map the original termination and test it.

• The linked issue's delivery scope is not met by this PR as submitted. It asks for a pinned first-party helper, adoption across compatible jobs, and the paired Docker-auth treatment. This changes one invocation while leaving the other repeated sites and auth work untouched. A preparatory slice can be reasonable, but it needs explicit maintainer agreement on the smaller scope rather than presenting this as the consolidation outcome. Please also coordinate with the contributor currently assigned to #6961.

Once the silent-success path and behavior-preservation gaps are fixed, the validation core should be in much better shape for reuse.

[souvikDevloper]

Thanks @cv — the silent-success path was a real bug and a bad one. Fixed in 3e77523.

Blocking: unsupported subcommands exited 0. Confirmed exactly as you reproduced: runx → 0, and no subcommand → 0. A typo would have skipped live E2E while the job reported success. The guard now rejects a missing or unsupported subcommand with a usage message and a non-zero exit, and — as you noted — the boundary check was equally fooled, so it now matches the subcommand as a whole word (...mts run(?![\w-])) instead of a substring. Regression tests drive the real CLI for runx, ru, RUN, --test-path, and the empty case, plus a boundary test that a runx workflow fails validation.

Termination behavior. Also right — spawnSync gives no numeric status on a signal death, and I collapsed that to 1, which made a killed/OOM-reaped run look like an ordinary test failure. resolveChildExitCode now maps a signal to 128+signo (matching the set -euo pipefail shell it replaced) and keeps spawn failures as a generic 1, with tests for each.

Scope + coordination with #6961's assignee. Agreed, and worth laying out the timeline so it's clear this wasn't a land-grab: I opened this PR on 2026-07-16, and @Shaurya2k06 was assigned #6961 on 2026-07-17. So this predates the assignment rather than cutting across it.

Usefully, the work turns out to be complementary, not duplicate:

• This PR: the live-Vitest invocation helper + validation, adopted in the canonical registry-targets job.
• @Shaurya2k06's refactor(ci): pin E2E Docker Hub auth to trusted setup action #7082: Docker Hub auth → trusted setup action — precisely the piece I deliberately left out, and via the trusted action mechanism I argued was the right one (I declined to move secret-bearing auth into a checkout-loaded script because that would make it branch-controllable).

@Shaurya2k06 — no conflict from my side; our diffs don't overlap (yours is the auth anchor, mine is live-vitest-invocation.mts + the one live job). Happy to work around you: if you'd rather own the live-Vitest half too, say so and I'll close this and hand you the helper + tests. Otherwise I'd suggest they land independently.

On the smaller scope: I'm not presenting this as the full consolidation outcome — you're right to push back on that framing. It's a preparatory slice, and it needs your explicit agreement rather than my assertion. If you'd prefer it not land as a partial, I'm equally happy to close it in favour of @Shaurya2k06's assigned work.

Signed-off-by: Souvik Ghosh 138186578+souvikDevloper@users.noreply.github.com

[wscurran]

✨ Thanks for the refactor. Centralizing and validating the live-Vitest invocation improves maintainability and security posture. Ready for maintainer review.

---

Related open issues:

• #6961 Consolidate repeated live E2E invocation and Docker-auth setup into pinned helper actions
• #6952 Extract executable GitHub Actions logic into tested helpers

[Shaurya2k06]

Thanks for confirming the Docker-auth piece, agreed, #7082 and this PR don't overlap.

On the live-Vitest half though: #6996 (merged 2026-07-16T15:22:59Z) already added tools/e2e/live-vitest-invocation.mts and adopted it across all ~70 qualifying jobs in e2e.yaml, not just the one job here. This PR is now showing CONFLICTING/DIRTY against main, which lines up with that — it's colliding with #6996's version of the same file rather than complementing it.

Given that, the live-Vitest half of #6961's Work list is already done via #6996. Worth closing this one rather than rebasing it, unless a rebase turns up something #6996 didn't cover, @cv, curious if you see anything #6984 still adds on top of #6996.

[apurvvkumaria]

Closing as superseded by merged #6996. That PR delivered the validated live-Vitest helper, adopted it across the 70 compatible jobs, preserved process exit/signal behavior, and added stricter malformed/repeated-option coverage. Merged #7079 separately delivered the trusted Docker-auth action portion of #6961. Current main therefore contains the intended behavior and a broader completed migration. Thank you for the focused initial implementation and tests.