Skip to content

fix(e2e): scope scenario advisor to scenario suite #4286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jyaunches wants to merge 1 commit into from
+196 −33
Closed

fix(e2e): scope scenario advisor to scenario suite #4286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 57 additions & 4 deletions test/e2e-scenario-advisor.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,14 @@ describe("E2E scenario advisor", () => {
expect(result.noScenarioE2eReason).toBeNull();
});

it("requires targeted scenario E2E when a validation suite changes", () => {
it("recognizes migrated test/e2e-scenario validation suite paths", () => {
const result = analyze([
"test/e2e/validation_suites/messaging/telegram/00-telegram-injection-safety.sh",
"test/e2e-scenario/validation_suites/messaging/telegram/00-telegram-injection-safety.sh",
]);

expect(result.relevantChangedFiles).toEqual([
"test/e2e-scenario/validation_suites/messaging/telegram/00-telegram-injection-safety.sh",
]);
expect(result.required).toContainEqual(
expect.objectContaining({
id: "ubuntu-repo-docker__cloud-nvidia-openclaw-telegram:messaging-telegram",
Expand All @@ -50,10 +53,48 @@ describe("E2E scenario advisor", () => {
);
});

it("requires all known scenarios when typed scenario internals change", () => {
const result = analyze(["test/e2e-scenario/scenarios/run.ts"]);

expect(result.required).toContainEqual(
expect.objectContaining({ id: "e2e-scenarios-all" }),
);
expect(result.required).toContainEqual(
expect.objectContaining({
scenario: "ubuntu-repo-docker__cloud-nvidia-openclaw",
}),
);
expect(result.required).toContainEqual(
expect.objectContaining({
scenario: "ubuntu-repo-docker__cloud-nvidia-openclaw-telegram",
}),
);
expect(result.required.length).toBeGreaterThan(7);
expect(result.noScenarioE2eReason).toBeNull();
});

it("requires all known scenarios when migrated manifests change", () => {
const result = analyze([
"test/e2e-scenario/manifests/openclaw-nvidia.yaml",
]);

expect(result.relevantChangedFiles).toEqual([
"test/e2e-scenario/manifests/openclaw-nvidia.yaml",
]);
expect(result.required).toContainEqual(
expect.objectContaining({ id: "e2e-scenarios-all" }),
);
expect(result.required).toContainEqual(
expect.objectContaining({
scenario: "ubuntu-repo-docker__cloud-nvidia-openclaw",
}),
);
});

it("requires all scenario E2E and targeted follow-up when suite metadata changes", () => {
const result = analyze([
"test/e2e/validation_suites/suites.yaml",
"test/e2e/validation_suites/messaging/telegram/00-telegram-injection-safety.sh",
"test/e2e-scenario/validation_suites/suites.yaml",
"test/e2e-scenario/validation_suites/messaging/telegram/00-telegram-injection-safety.sh",
]);

expect(result.required).toContainEqual(
Expand All @@ -76,6 +117,18 @@ describe("E2E scenario advisor", () => {
expect(result.noScenarioE2eReason).toMatch(/No scenario workflow/);
});

it("does not recommend scenario E2E for legacy test/e2e paths", () => {
const result = analyze([
"test/e2e/validation_suites/messaging/telegram/00-telegram-injection-safety.sh",
"test/e2e/runtime/run-scenario.sh",
]);

expect(result.relevantChangedFiles).toEqual([]);
expect(result.required).toEqual([]);
expect(result.optional).toEqual([]);
expect(result.noScenarioE2eReason).toMatch(/No scenario workflow/);
});

it("renders a summary and second sticky scenario comment", () => {
const result = analyze([".github/workflows/e2e-scenarios.yaml"]);
const summary = renderScenarioSummary(result);
Expand Down
168 changes: 139 additions & 29 deletions tools/e2e-advisor/scenarios.mts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import { parseArgs, writeJson } from "../advisors/io.mts";
const SCENARIO_WORKFLOW = "e2e-scenarios.yaml";
const SCENARIO_ALL_WORKFLOW = "e2e-scenarios-all.yaml";
const DEFAULT_BASELINE_SCENARIO = "ubuntu-repo-cloud-openclaw";
const SCENARIO_ROOT = "test/e2e-scenario";
const SCENARIO_DATA_ROOTS = [SCENARIO_ROOT, "test/e2e"];
const CORE_SCENARIO_IDS = [
"ubuntu-repo-cloud-openclaw",
"ubuntu-repo-cloud-hermes",
Expand Down Expand Up @@ -104,33 +106,47 @@ export function analyzeScenarioRecommendations({
const reasons = new Set<string>();
const relevantChangedFiles = changedFiles.filter(isScenarioRelevantFile);
let allScenariosRequired = false;
let allKnownScenariosRequired = false;
const scenarioWorkflowInputs = detectScenarioWorkflowInputs(root);

for (const file of changedFiles) {
const scenarioPath = parseScenarioPath(file);
if (file === ".github/workflows/e2e-scenarios-all.yaml") {
allScenariosRequired = true;
reasons.add("the all-scenarios fan-out workflow changed");
} else if (file === ".github/workflows/e2e-scenarios.yaml") {
allScenariosRequired = true;
reasons.add("the reusable single-scenario workflow changed");
} else if (file === "test/e2e/nemoclaw_scenarios/scenarios.yaml") {
} else if (scenarioPath?.kind === "scenario-catalog") {
allScenariosRequired = true;
allKnownScenariosRequired = true;
reasons.add("scenario catalog metadata changed");
} else if (file === "test/e2e/nemoclaw_scenarios/expected-states.yaml") {
} else if (scenarioPath?.kind === "expected-states") {
allScenariosRequired = true;
allKnownScenariosRequired = true;
reasons.add("expected-state metadata changed");
} else if (file === "test/e2e/validation_suites/suites.yaml") {
} else if (scenarioPath?.kind === "suite-catalog") {
allScenariosRequired = true;
allKnownScenariosRequired = true;
reasons.add("suite catalog metadata changed");
} else if (
file.startsWith("test/e2e/runtime/") ||
file.startsWith("test/e2e/nemoclaw_scenarios/helpers/")
) {
} else if (scenarioPath?.kind === "shared-runtime") {
allScenariosRequired = true;
allKnownScenariosRequired = true;
reasons.add("shared scenario runner/runtime code changed");
} else if (
file.startsWith("test/e2e/nemoclaw_scenarios/onboard/") ||
file.startsWith("test/e2e/nemoclaw_scenarios/install/")
) {
} else if (scenarioPath?.kind === "typed-scenario-code") {
allScenariosRequired = true;
allKnownScenariosRequired = true;
reasons.add("typed scenario builder/assertion/compiler code changed");
} else if (scenarioPath?.kind === "manifest") {
allScenariosRequired = true;
allKnownScenariosRequired = true;
reasons.add("scenario onboarding manifest changed");
} else if (scenarioPath?.kind === "onboarding-assertion") {
allScenariosRequired = true;
allKnownScenariosRequired = true;
reasons.add("onboarding assertion changed");
} else if (scenarioPath?.kind === "install-onboard-helper") {
allScenariosRequired = true;
directScenarioIds.add(DEFAULT_BASELINE_SCENARIO);
Comment on lines +148 to 150

@coderabbitai coderabbitai Bot May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't skip scenarios that were explicitly added to directScenarioIds.

The continue on Line 189 drops the baseline scenario queued on Line 150 and the core scenarios queued on Lines 169-171. That means install/onboard-helper changes lose their targeted baseline run, and broad metadata changes omit direct recommendations for core scenarios.

Possible fix 
   for (const scenario of [...directScenarioIds].sort()) {
-    if (allScenariosRequired && CORE_SCENARIO_IDS.includes(scenario)) continue;
+    const skipBecauseCoveredByFanout =
+      allScenariosRequired &&
+      !allKnownScenariosRequired &&
+      scenario !== DEFAULT_BASELINE_SCENARIO &&
+      CORE_SCENARIO_IDS.includes(scenario);
+    if (skipBecauseCoveredByFanout) continue;
     const suiteFilter = suiteFilterForScenario(
       scenario,
       changedSuiteIds,
       scenarios,

Also applies to: 169-171, 188-200

🤖 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-advisor/scenarios.mts` around lines 148 - 150, The code is skipping
scenarios that were explicitly added to directScenarioIds because a later
continue bypasses the run logic; modify the control flow in the block handling
scenarioPath (specifically where DEFAULT_BASELINE_SCENARIO and core scenarios
are added to directScenarioIds and allScenariosRequired is set) so that you do
not early-continue and drop those IDs — either remove the continue or move the
continue below/after the code that enqueues IDs, and ensure the same change is
applied to the other similar branches (the blocks around where core scenarios
are added and the branch covering lines 188-200) so directScenarioIds additions
are always preserved before skipping further processing.

reasons.add("scenario install/onboard helper code changed");
}
Expand All @@ -150,6 +166,11 @@ export function analyzeScenarioRecommendations({
for (const scenario of matchingScenarios) directScenarioIds.add(scenario);
}

if (allKnownScenariosRequired) {
for (const scenario of Object.keys(scenarios))
directScenarioIds.add(scenario);
}

const required: ScenarioRecommendation[] = [];
const optional: ScenarioRecommendation[] = [];
if (allScenariosRequired) {
Expand All @@ -176,6 +197,7 @@ export function analyzeScenarioRecommendations({
scenario,
suiteFilter,
reasonForScenario(scenario, changedSuiteIds, reasons),
scenarioWorkflowInputs,
),
);
}
Expand All @@ -196,6 +218,7 @@ export function analyzeScenarioRecommendations({
scenario,
suiteFilter,
`Targeted follow-up for changed suite(s): ${suiteFilter || [...changedSuiteIds].sort().join(",")}`,
scenarioWorkflowInputs,
false,
),
);
Expand All @@ -216,6 +239,7 @@ export function analyzeScenarioRecommendations({
specialScenario,
suiteFilterForScenario(specialScenario, changedSuiteIds, scenarios),
"Special-runner scenario covers a changed suite but may require scarce hardware/secrets.",
scenarioWorkflowInputs,
false,
),
);
Expand Down Expand Up @@ -279,11 +303,11 @@ export function renderScenarioSummary(result: ScenarioAdvisorResult): string {
}

function loadScenarios(root: string): Record<string, ScenarioEntry> {
const filePath = path.join(
const filePath = firstExistingScenarioPath(
root,
"test/e2e/nemoclaw_scenarios/scenarios.yaml",
"nemoclaw_scenarios/scenarios.yaml",
);
if (!fs.existsSync(filePath)) return {};
if (!filePath) return {};
const text = fs.readFileSync(filePath, "utf8");
return {
...parseScenarioSection(text, "test_plans"),
Expand All @@ -292,11 +316,25 @@ function loadScenarios(root: string): Record<string, ScenarioEntry> {
}

function loadSuiteScriptMap(root: string): Record<string, string[]> {
const filePath = path.join(root, "test/e2e/validation_suites/suites.yaml");
if (!fs.existsSync(filePath)) return {};
const filePath = firstExistingScenarioPath(
root,
"validation_suites/suites.yaml",
);
if (!filePath) return {};
return parseSuiteScripts(fs.readFileSync(filePath, "utf8"));
}

function firstExistingScenarioPath(
root: string,
relative: string,
): string | undefined {
for (const scenarioRoot of SCENARIO_DATA_ROOTS) {
const filePath = path.join(root, scenarioRoot, relative);
if (fs.existsSync(filePath)) return filePath;
}
return undefined;
Comment on lines +327 to +335

@coderabbitai coderabbitai Bot May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve the flat metadata fallback in firstExistingScenarioPath().

This helper only probes the nested nemoclaw_scenarios/... and validation_suites/... layout. The runtime resolver also accepts flat fixtures like <e2e-root>/scenarios.yaml, expected-states.yaml, and suites.yaml, so those valid layouts resolve to empty metadata here and the advisor loses suite/scenario targeting.

Possible fix 
 function firstExistingScenarioPath(
   root: string,
   relative: string,
 ): string | undefined {
+  const flatFallback =
+    relative === "nemoclaw_scenarios/scenarios.yaml"
+      ? "scenarios.yaml"
+      : relative === "nemoclaw_scenarios/expected-states.yaml"
+        ? "expected-states.yaml"
+        : relative === "validation_suites/suites.yaml"
+          ? "suites.yaml"
+          : undefined;
+
   for (const scenarioRoot of SCENARIO_DATA_ROOTS) {
-    const filePath = path.join(root, scenarioRoot, relative);
-    if (fs.existsSync(filePath)) return filePath;
+    const candidates = [path.join(root, scenarioRoot, relative)];
+    if (flatFallback) {
+      candidates.push(path.join(root, scenarioRoot, flatFallback));
+    }
+    for (const filePath of candidates) {
+      if (fs.existsSync(filePath)) return filePath;
+    }
   }
   return undefined;
 }
🤖 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-advisor/scenarios.mts` around lines 327 - 335, The helper
firstExistingScenarioPath currently only checks nested SCENARIO_DATA_ROOTS paths
and therefore ignores flat metadata files at the repository root; modify
firstExistingScenarioPath to also probe the flat path (path.join(root,
relative)) before iterating SCENARIO_DATA_ROOTS (or include it in the iteration)
so files like scenarios.yaml, expected-states.yaml, and suites.yaml at the e2e
root are detected and returned by the function.

}

function parseScenarioSection(
text: string,
sectionName: string,
Expand Down Expand Up @@ -413,23 +451,75 @@ function isScenarioRelevantFile(file: string): boolean {
return (
file === ".github/workflows/e2e-scenarios.yaml" ||
file === ".github/workflows/e2e-scenarios-all.yaml" ||
file.startsWith("test/e2e/runtime/") ||
file.startsWith("test/e2e/nemoclaw_scenarios/") ||
file.startsWith("test/e2e/validation_suites/")
parseScenarioPath(file) !== undefined
);
}

type ScenarioPathKind =
| "scenario-catalog"
| "expected-states"
| "suite-catalog"
| "validation-suite"
| "shared-runtime"
| "install-onboard-helper"
| "typed-scenario-code"
| "manifest"
| "onboarding-assertion"
| "scenario-docs-or-tests";

function parseScenarioPath(
file: string,
): { root: string; relative: string; kind: ScenarioPathKind } | undefined {
const scenarioRoot = file.startsWith(`${SCENARIO_ROOT}/`)
? SCENARIO_ROOT
: undefined;
if (!scenarioRoot) return undefined;
const relative = file.slice(scenarioRoot.length + 1);

if (relative === "nemoclaw_scenarios/scenarios.yaml")
return { root: scenarioRoot, relative, kind: "scenario-catalog" };
if (relative === "nemoclaw_scenarios/expected-states.yaml")
return { root: scenarioRoot, relative, kind: "expected-states" };
if (relative === "validation_suites/suites.yaml")
return { root: scenarioRoot, relative, kind: "suite-catalog" };
if (
relative.startsWith("runtime/") ||
relative.startsWith("nemoclaw_scenarios/helpers/")
) {
return { root: scenarioRoot, relative, kind: "shared-runtime" };
}
if (
relative.startsWith("nemoclaw_scenarios/onboard/") ||
relative.startsWith("nemoclaw_scenarios/install/")
) {
return { root: scenarioRoot, relative, kind: "install-onboard-helper" };
}
if (relative.startsWith("validation_suites/"))
return { root: scenarioRoot, relative, kind: "validation-suite" };
if (relative.startsWith("scenarios/"))
return { root: scenarioRoot, relative, kind: "typed-scenario-code" };
if (relative.startsWith("manifests/"))
return { root: scenarioRoot, relative, kind: "manifest" };
if (relative.startsWith("onboarding_assertions/"))
return { root: scenarioRoot, relative, kind: "onboarding-assertion" };
if (
relative.startsWith("framework-tests/") ||
relative.startsWith("scenario-framework-tests/") ||
relative.startsWith("docs/")
) {
return { root: scenarioRoot, relative, kind: "scenario-docs-or-tests" };
}
return undefined;
}

function inferSuiteIdsFromPath(
file: string,
suiteIds: Set<string>,
suiteScriptMap: Record<string, string[]>,
): string[] {
if (
!file.startsWith("test/e2e/validation_suites/") ||
file.endsWith("/suites.yaml")
)
return [];
const relative = file.slice("test/e2e/validation_suites/".length);
const scenarioPath = parseScenarioPath(file);
if (scenarioPath?.kind !== "validation-suite") return [];
const relative = scenarioPath.relative.slice("validation_suites/".length);
const segments = relative.split("/");
const candidates = new Set<string>();
for (let size = Math.min(segments.length, 3); size >= 1; size -= 1) {
Expand Down Expand Up @@ -507,23 +597,43 @@ function reasonForScenario(
return `Scenario \`${scenario}\` exercises the changed scenario E2E surface.${suiteText} ${[...reasons].join("; ")}`.trim();
}

type ScenarioWorkflowInputs = {
scenarioField: "scenario" | "scenarios";
supportsSuiteFilter: boolean;
};

function detectScenarioWorkflowInputs(root: string): ScenarioWorkflowInputs {
const workflowPath = path.join(root, ".github/workflows/e2e-scenarios.yaml");
const text = fs.existsSync(workflowPath)
? fs.readFileSync(workflowPath, "utf8")
: "";
return {
scenarioField: /^\s{6}scenarios:\s*$/m.test(text)
? "scenarios"
: "scenario",
supportsSuiteFilter: /^\s{6}suite_filter:\s*$/m.test(text),
};
}

function buildSingleScenarioRecommendation(
scenario: string,
suiteFilter: string | undefined,
reason: string,
workflowInputs: ScenarioWorkflowInputs,
required = true,
): ScenarioRecommendation {
const suitePart = suiteFilter
? ` --field suite_filter=${shellQuote(suiteFilter)}`
: "";
const suitePart =
suiteFilter && workflowInputs.supportsSuiteFilter
? ` --field suite_filter=${shellQuote(suiteFilter)}`
: "";
return {
id: suiteFilter ? `${scenario}:${suiteFilter}` : scenario,
workflow: SCENARIO_WORKFLOW,
scenario,
suiteFilter,
required,
reason,
dispatchCommand: `gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenario=${shellQuote(scenario)}${suitePart}`,
dispatchCommand: `gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field ${workflowInputs.scenarioField}=${shellQuote(scenario)}${suitePart}`,
};
}

Expand Down
Loading