test(e2e): add scenario aliases for layered plans by cv · Pull Request #4651 · NVIDIA/NemoClaw

cv · 2026-06-02T06:55:06Z

Summary

Add friendly setup-scenario aliases for the layered E2E onboarding test plans so the YAML shell resolver can run every canonical typed scenario ID. This closes the drift where workflow/advisor scenario IDs resolved through the typed registry but failed through runtime/run-scenario.sh.

Related Issue

Fixes #4378.

Changes

Add 12 missing setup_scenarios aliases in test/e2e-scenario/nemoclaw_scenarios/scenarios.yaml for messaging, Hermes messaging, lifecycle, token-rotation, Brave, and OpenAI-compatible plans.
Preserve dimensions, expected-state, and suite metadata on each alias so coverage reports stay populated.
Add a resolver regression test that every typed canonical scenario ID resolves through YAML setup scenarios.

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)

Additional validation:

bash test/e2e-scenario/runtime/run-scenario.sh <new-friendly-id> --plan-only passed for all 12 added aliases.
npx vitest run --project e2e-scenario-framework --silent=false --reporter=default passes.

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

Summary by CodeRabbit

Tests
- Added comprehensive end-to-end test to validate scenario resolution across all registered scenarios.
Chores
- Expanded end-to-end testing scenarios to cover additional OpenCLAW onboarding variants, cloud deployment options (Brave, Telegram, Discord, Slack), and lifecycle scenarios (resume-after-interrupt, repair-existing-config, token-rotation).

coderabbitai · 2026-06-02T06:55:19Z

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: 5445d80a-d127-4d13-9c66-97f4351c9e6b

📥 Commits

Reviewing files that changed from the base of the PR and between 95ad201 and dbc672f.

📒 Files selected for processing (2)

test/e2e-scenario/framework-tests/e2e-scenario-resolver.test.ts
test/e2e-scenario/nemoclaw_scenarios/scenarios.yaml

📝 Walkthrough

Walkthrough

This PR adds 13 user-friendly scenario aliases in YAML for existing OpenCLAW/Hermes test plans and introduces a validation test to confirm all registered scenarios resolve without errors.

Changes

Scenario Aliases & Resolution Validation

Layer / File(s)	Summary
Setup scenario aliases `test/e2e-scenario/nemoclaw_scenarios/scenarios.yaml`	Adds 13 `setup_scenarios` entries mapping to existing `test_plans` IDs: ubuntu-repo-openai-compatible-openclaw, cloud variants for Brave/Telegram/Discord/Slack, Hermes Discord/Slack, and lifecycle variants (resume-after-interrupt, repair-existing-config, double-same-provider, double-provider-switch, token-rotation).
Scenario resolution validator `test/e2e-scenario/framework-tests/e2e-scenario-resolver.test.ts`	Imports `listScenarios` and adds a test that enumerates all scenario IDs, resolves each via `resolveScenario`, collects errors as `<id>: <message>`, and asserts the failures list is empty.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 In YAML fields where scenarios grow,
Thirteen aliases put on a show,
Each variant tested, each plan resolved true,
From OpenCLAW clouds to Hermes anew!
The validator hops, checks none astray,
And all the scenarios dance the right way. 🎉

🚥 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	Title references 'scenario aliases for layered plans' which directly matches the core change: adding setup_scenario aliases for layered E2E test plans.
Linked Issues check	✅ Passed	PR fulfills all coding requirements: adds 12 missing setup_scenarios aliases in scenarios.yaml with alias_for_plan mappings, adds regression test to enumerate and resolve all scenario IDs, preserves metadata (expected_state/suites) on aliases, and framework tests pass.
Out of Scope Changes check	✅ Passed	All changes are scoped to the linked issue: test file additions validate scenario resolution, and scenarios.yaml additions are the required aliases with no unrelated modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch test/e2e-scenario-friendly-aliases

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

github-actions · 2026-06-02T06:59:37Z

PR Review Advisor

Findings: 1 needs attention, 3 worth checking, 0 nice ideas
Top item: Advisor acceptance clauses are not covered by the new test

Review findings

🛠️ Needs attention

Advisor acceptance clauses are not covered by the new resolver test (test/e2e-scenario/framework-tests/e2e-scenario-resolver.test.ts:70): Linked issue test(e2e): add setup_scenarios aliases for 13 layered test_plans (epic #3588 Phase 4) #4378 explicitly requires `tools/e2e-advisor/scenarios.mts` to enumerate these IDs from YAML and `test/e2e-scenario-advisor.test.ts` to pass against the YAML resolver. This PR adds a framework resolver test that uses the typed TS registry (`listScenarios()`), but it does not change or test the advisor path, and `test/e2e-scenario-advisor.test.ts` has no YAML resolver or `ubuntu-repo-cloud-openclaw-telegram` coverage.
- Recommendation: Add or update advisor-focused coverage that exercises the YAML resolver for at least the previously failing friendly ID, or narrow/update the linked acceptance criteria if that work is intentionally out of scope.
- Evidence: The new test imports `listScenarios` from `../scenarios/registry.ts` and calls `resolveScenario(scenario.id, meta)`. Nearby advisor files show no deterministic YAML enumeration change in `tools/e2e-advisor/scenarios.mts`, and grep found no `resolveScenario` or `ubuntu-repo-cloud-openclaw-telegram` assertion in `test/e2e-scenario-advisor.test.ts`.

🔎 Worth checking

Source-of-truth review needed: `setup_scenarios` `alias_for_plan` bridge for typed friendly IDs: The advisor marked localized patch analysis as needs_followup.
- Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
- Evidence: The diff adds aliased `setup_scenarios` entries and a test that imports `listScenarios` from the typed registry, preserving a migration bridge rather than fully eliminating the dual source.
Alias entries duplicate metadata that the resolver does not consistently use or validate (test/e2e-scenario/nemoclaw_scenarios/scenarios.yaml:217): The new `setup_scenarios` aliases copy `dimensions`, `expected_state`, and `suites` next to `alias_for_plan`, but the resolver generally resolves aliased scenarios from the target layered `test_plans` entry, and loader validation skips detailed checks once `alias_for_plan` is present. That makes the copied fields a second source of truth that can drift silently from the actual plan used by `run-scenario.sh`.
- Recommendation: Either keep aliased setup entries minimal and document that the `test_plans` entry is authoritative, or add a parity test that compares each alias's copied fields against its target layered plan and the typed scenario metadata that still exists during migration.
- Evidence: `load.ts` continues when `typeof e.alias_for_plan === "string"`; `resolveScenario` sets `planId = legacy?.alias_for_plan ?? scenarioId` and then uses `layeredPlan?.expected_state` and `layeredPlan?.suites` ahead of legacy fields. The new no-throw test would not fail if the duplicated alias `suites` or `expected_state` drifted from the target plan.
Resolver coverage does not assert security-sensitive metadata parity (test/e2e-scenario/framework-tests/e2e-scenario-resolver.test.ts:70): The new regression test proves friendly typed IDs resolve, but it does not assert that credential-gating metadata, suite selections, runner requirements, or manifests remain consistent across the typed registry, YAML plans, and manifests. For scenarios that require external provider tokens, silent drift can reduce E2E coverage or omit credential-gated validation without this test catching it.
- Recommendation: Extend the regression coverage to compare security-relevant metadata such as required secret names, selected suites, runner requirements, and onboarding assertions for aliased scenarios, or add a separate metadata parity test for those fields.
- Evidence: The test only collects exceptions from `resolveScenario(scenario.id, meta)` and expects `failures` to equal `[]`; it does not inspect `plan.required_secrets`, suite parity, runner requirements, or manifest credential refs.

🌱 Nice ideas

None.

Workflow run details

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

github-actions · 2026-06-02T07:06:38Z

E2E Advisor Recommendation

Required E2E: None
Optional E2E: e2e-scenarios-dry-run-new-ubuntu-catalog-scenarios

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

None. No required E2E is recommended: this PR changes only E2E metadata and E2E framework tests, with no runtime/user-flow implementation changes. Standard unit/framework tests should validate the resolver and YAML consistency; live installer/onboarding/sandbox/inference E2E would not add merge-blocking signal for these files.

Optional E2E

e2e-scenarios-dry-run-new-ubuntu-catalog-scenarios (medium): Optional confidence check: dispatch the existing scenario runner for the newly cataloged Ubuntu scenarios to validate typed plan compilation, runner routing, manifest compatibility, and dry-run assertion wiring. This is not merge-blocking because the PR only changes test metadata and resolver tests.

New E2E recommendations

scenario-catalog-parity (low): There is no dedicated lightweight workflow-dispatched check that validates legacy YAML scenario catalog parity against the typed registry whenever nemoclaw_scenarios/scenarios.yaml changes; coverage currently relies on framework Vitest tests.
- Suggested test: scenario-catalog-parity-validation

github-actions · 2026-06-02T07:06:39Z

E2E Scenario Advisor Recommendation

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

Dispatch required scenario E2E:

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

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

e2e-scenarios-all: Scenario catalog metadata changed in test/e2e-scenario/nemoclaw_scenarios/scenarios.yaml, adding multiple setup_scenarios/onboarding profiles. Per policy, scenario metadata changes require the full scenario fan-out to validate all dispatchable scenario plans and coverage.
- Dispatch: gh workflow run e2e-scenarios-all.yaml --ref <pr-head-ref>

Optional scenario E2E

None.

Relevant changed files

test/e2e-scenario/framework-tests/e2e-scenario-resolver.test.ts
test/e2e-scenario/nemoclaw_scenarios/scenarios.yaml

cjagwani

LGTM

wscurran · 2026-06-03T18:09:09Z

✨
Related open issues:

#4378 test(e2e): add setup_scenarios aliases for 13 layered test_plans (epic #3588 Phase 4)

test(e2e): add scenario aliases for layered plans

dbc672f

cv self-assigned this Jun 2, 2026

cv requested a review from jyaunches June 2, 2026 07:06

cv requested review from cjagwani, jyaunches, prekshivyas and sandl99 and removed request for jyaunches June 2, 2026 07:06

cv added the v0.0.58 Release target label Jun 3, 2026

cjagwani approved these changes Jun 3, 2026

View reviewed changes

cv merged commit 0c3ee73 into main Jun 3, 2026
32 checks passed

cv deleted the test/e2e-scenario-friendly-aliases branch June 3, 2026 17:50

wscurran added area: e2e End-to-end tests, nightly failures, or validation infrastructure feature PR adds or expands user-visible functionality labels Jun 3, 2026

This was referenced Jun 3, 2026

docs(e2e): reconcile scenario migration docs #4654

Merged

test(e2e): execute real shell assertions; delete dry-run, --validate-only, and the bash runner #4380

Merged

Conversation

cv commented Jun 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Type of Change

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

github-actions Bot commented Jun 2, 2026

PR Review Advisor

🛠️ Needs attention

🔎 Worth checking

🌱 Nice ideas

Uh oh!

github-actions Bot commented Jun 2, 2026

E2E Advisor Recommendation

E2E Recommendation Advisor

Required E2E

Optional E2E

New E2E recommendations

Uh oh!

github-actions Bot commented Jun 2, 2026

E2E Scenario Advisor Recommendation

E2E Scenario Advisor

Required scenario E2E

Optional scenario E2E

Relevant changed files

Uh oh!

cjagwani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wscurran commented Jun 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cv commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading