Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

# ── Tests ──
/test/ @NVIDIA/nemoclaw-engineer
/test/e2e/mock-parity.json @NVIDIA/nemoclaw-maintainer @NVIDIA/nemoclaw-engineer

# ── CI / GitHub config ──
/.github/ @NVIDIA/nemoclaw-maintainer
Expand Down
49 changes: 35 additions & 14 deletions .github/workflows/e2e-advisor.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ name: E2E / Advisor
on:
pull_request:
types: [opened, synchronize, reopened, ready_for_review]
# Fork PRs cannot access advisor secrets in pull_request context. This
# parallel path executes the workflow from trusted main and treats the PR
# pull ref only as inert analysis data.
pull_request_target:
types: [opened, synchronize, reopened, ready_for_review]
workflow_dispatch:
inputs:
base_ref:
Expand Down Expand Up @@ -47,16 +52,15 @@ permissions:
# by integration" despite `issues: write`. See the comment step below and
# https://github.com/orgs/community/discussions/56632.
pull-requests: write
issues: write

concurrency:
group: e2e-advisor-${{ github.event.pull_request.number || github.ref }}
group: e2e-advisor-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true

jobs:
advise:
name: E2E recommendation
if: ${{ github.repository == 'NVIDIA/NemoClaw' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == 'NVIDIA/NemoClaw') }}
if: ${{ github.repository == 'NVIDIA/NemoClaw' && (github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == 'NVIDIA/NemoClaw') || (github.event_name == 'pull_request_target' && github.event.pull_request.head.repo.full_name != 'NVIDIA/NemoClaw')) }}
runs-on: ubuntu-latest
timeout-minutes: 20
env:
Expand Down Expand Up @@ -121,11 +125,12 @@ jobs:
run: echo "ADVISOR_WORKDIR=$GITHUB_WORKSPACE/pr-workdir" >> "$GITHUB_ENV"

- name: Prepare target PR checkout
if: ${{ github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '' }}
if: ${{ github.event_name == 'pull_request_target' || (github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '') }}
env:
TARGET_REPO: ${{ inputs.target_repo }}
TARGET_PR: ${{ inputs.target_pr }}
TARGET_BASE: ${{ inputs.target_base }}
TARGET_REPO: ${{ github.event_name == 'pull_request_target' && github.repository || inputs.target_repo }}
TARGET_PR: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.number || inputs.target_pr }}
TARGET_BASE: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.base.ref || inputs.target_base }}
EXPECTED_HEAD_SHA: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || '' }}
run: |
if [[ ! "$TARGET_REPO" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]]; then
echo "::error::target_repo must match owner/repo with GitHub-safe characters"
Expand All @@ -144,10 +149,26 @@ jobs:
mkdir -p "$TARGET_DIR"
git -C "$TARGET_DIR" init
git -C "$TARGET_DIR" remote add target "https://github.com/${TARGET_REPO}.git"
git -C "$TARGET_DIR" fetch --no-tags target "$TARGET_BASE"
git -C "$TARGET_DIR" fetch --no-tags target "refs/heads/${TARGET_BASE}:refs/remotes/target/${TARGET_BASE}"
git -C "$TARGET_DIR" fetch --no-tags target "pull/${TARGET_PR}/head:refs/remotes/target/pr-${TARGET_PR}"
git -C "$TARGET_DIR" checkout --detach "refs/remotes/target/pr-${TARGET_PR}"
if [ -n "$EXPECTED_HEAD_SHA" ] && [ "$(git -C "$TARGET_DIR" rev-parse HEAD)" != "$EXPECTED_HEAD_SHA" ]; then
echo "::error::Fetched pull ref does not match the triggering PR head SHA"
exit 1
fi
echo "ADVISOR_WORKDIR=$TARGET_DIR" >> "$GITHUB_ENV"
echo "PR_NUMBER=$TARGET_PR" >> "$GITHUB_ENV"

# The advisor reads repository files while holding its model API key.
# Remove worktree symlinks so untrusted PR data cannot redirect a read
# to runner files such as /proc/self/environ. Git diff still reads the
# committed objects, so symlink additions and changes remain visible.
- name: Remove symlinks from analysis workspace
shell: bash
run: |
while IFS= read -r -d '' link; do
rm -- "$link"
done < <(find "$ADVISOR_WORKDIR" -type l -print0)

# Pinned SDK install. The version is held in PI_SDK_VERSION above so
# the pin is reviewed as a code change, not silently inherited from
Expand All @@ -164,8 +185,8 @@ jobs:
id: analysis
continue-on-error: true
env:
BASE_REF: ${{ github.event_name == 'pull_request' && format('origin/{0}', github.base_ref) || (github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '' && format('target/{0}', inputs.target_base) || inputs.base_ref) }}
HEAD_REF: ${{ github.event_name == 'pull_request' && 'HEAD' || (github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '' && 'HEAD' || inputs.head_ref) }}
BASE_REF: ${{ github.event_name == 'pull_request' && format('origin/{0}', github.base_ref) || (github.event_name == 'pull_request_target' && format('target/{0}', github.event.pull_request.base.ref) || (github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '' && format('target/{0}', inputs.target_base) || inputs.base_ref)) }}
HEAD_REF: ${{ (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && 'HEAD' || (github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '' && 'HEAD' || inputs.head_ref) }}
E2E_ADVISOR_RUN_ANALYSIS: ${{ github.event_name == 'workflow_dispatch' && inputs.run_analysis == false && '0' || '1' }}
# Preferred E2E advisor secret.
E2E_ADVISOR_API_KEY: ${{ secrets.PI_E2E_ADVISOR_API_KEY }}
Expand All @@ -181,8 +202,8 @@ jobs:
id: target-analysis
continue-on-error: true
env:
BASE_REF: ${{ github.event_name == 'pull_request' && format('origin/{0}', github.base_ref) || (github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '' && format('target/{0}', inputs.target_base) || inputs.base_ref) }}
HEAD_REF: ${{ github.event_name == 'pull_request' && 'HEAD' || (github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '' && 'HEAD' || inputs.head_ref) }}
BASE_REF: ${{ github.event_name == 'pull_request' && format('origin/{0}', github.base_ref) || (github.event_name == 'pull_request_target' && format('target/{0}', github.event.pull_request.base.ref) || (github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '' && format('target/{0}', inputs.target_base) || inputs.base_ref)) }}
HEAD_REF: ${{ (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && 'HEAD' || (github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '' && 'HEAD' || inputs.head_ref) }}
E2E_TARGET_ADVISOR_RUN_ANALYSIS: ${{ github.event_name == 'workflow_dispatch' && inputs.run_analysis == false && '0' || '1' }}
# Reuse the shared E2E advisor secret. The target advisor is a
# separate prompt/agent but uses the same model and credential.
Expand All @@ -208,7 +229,7 @@ jobs:
fi

- name: Post E2E advisor PR comment
if: ${{ always() && github.event_name == 'pull_request' }}
if: ${{ always() && (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') }}
continue-on-error: true
env:
GH_TOKEN: ${{ secrets.E2E_ADVISOR_GITHUB_TOKEN || github.token }}
Expand All @@ -229,7 +250,7 @@ jobs:
fi

- name: Post E2E target advisor PR comment
if: ${{ always() && github.event_name == 'pull_request' }}
if: ${{ always() && (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') }}
continue-on-error: true
env:
GH_TOKEN: ${{ secrets.E2E_ADVISOR_GITHUB_TOKEN || github.token }}
Expand Down
11 changes: 11 additions & 0 deletions .github/workflows/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ jobs:
- name: Checkout
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
fetch-depth: 0
persist-credentials: false

- name: Setup Node.js
Expand All @@ -169,6 +170,16 @@ jobs:
- name: Install E2E support dependencies
run: npm ci --ignore-scripts

- name: Validate changed live E2E mock parity
env:
BASE_SHA: ${{ github.event.before }}
run: |
if [ "$BASE_SHA" = "0000000000000000000000000000000000000000" ]; then
echo "Skipping changed live E2E parity: main has no prior commit."
exit 0
fi
npx tsx scripts/checks/e2e-mock-parity.ts --base "$BASE_SHA" --head HEAD

Comment thread
coderabbitai[bot] marked this conversation as resolved.
- name: Build CLI artifacts for E2E support
run: npm run build:cli

Expand Down
7 changes: 7 additions & 0 deletions .github/workflows/pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ jobs:
- name: Checkout
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
fetch-depth: 0
persist-credentials: false

- name: Setup Node.js
Expand All @@ -328,6 +329,12 @@ jobs:
- name: Install E2E support dependencies
run: npm ci --ignore-scripts

- name: Validate changed live E2E mock parity
# The pull_request payload can retain an older base SHA while GitHub's
# checked-out merge ref already targets newer main. Diff the merge
# parents so base-only changes are never attributed to the PR.
run: npx tsx scripts/checks/e2e-mock-parity.ts --base HEAD^1 --head HEAD^2

- name: Build CLI artifacts for E2E support
run: npm run build:cli

Expand Down
146 changes: 146 additions & 0 deletions scripts/checks/e2e-mock-parity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import { execFileSync } from "node:child_process";
import fs from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";

const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../..");
export const DEFAULT_PARITY_MANIFEST = "test/e2e/mock-parity.json";

export type MockParityEntry = {
live: string;
fast?: string[];
liveOnlyReason?: string;
};

export type MockParityManifest = {
version: 1;
entries: MockParityEntry[];
};

const LIVE_TEST = /^test\/e2e\/live\/.+\.test\.ts$/u;
const FAST_TESTS = [
/^src\/.+\.test\.ts$/u,
/^nemoclaw\/src\/.+\.test\.ts$/u,
/^test\/e2e\/support\/.+\.test\.ts$/u,
/^test\/(?!e2e\/|package-contract\/).+\.test\.(?:js|ts)$/u,
] as const;

function isSafeRepoPath(file: string): boolean {
return (
file.length > 0 &&
!path.posix.isAbsolute(file) &&
!file.includes("\\") &&
!file.split("/").includes("..")
);
}

function isFastPrTest(file: string): boolean {
return isSafeRepoPath(file) && FAST_TESTS.some((pattern) => pattern.test(file));
}

export function validateMockParity(options: {
manifest: MockParityManifest;
changedFiles: readonly string[];
fileExists?: (file: string) => boolean;
}): string[] {
const {
manifest,
changedFiles,
fileExists = (file) => fs.existsSync(path.join(REPO_ROOT, file)),
} = options;
const errors: string[] = [];

if (manifest.version !== 1 || !Array.isArray(manifest.entries)) {
return ["mock parity manifest must have version 1 and an entries array"];
}

const entries = new Map<string, MockParityEntry>();
for (const entry of manifest.entries) {
if (!entry || typeof entry !== "object" || typeof entry.live !== "string") {
errors.push("mock parity entries must be objects with a live path");
continue;
}
if (!isSafeRepoPath(entry.live) || !LIVE_TEST.test(entry.live)) {
errors.push(`${entry.live}: live path must be a test/e2e/live/**/*.test.ts file`);
continue;
}
if (entries.has(entry.live)) {
errors.push(`${entry.live}: duplicate mock parity entry`);
continue;
}
entries.set(entry.live, entry);

if (
entry.fast !== undefined &&
(!Array.isArray(entry.fast) || entry.fast.some((file) => typeof file !== "string"))
) {
errors.push(`${entry.live}: fast must be an array of test paths`);
continue;
}
if (entry.liveOnlyReason !== undefined && typeof entry.liveOnlyReason !== "string") {
errors.push(`${entry.live}: liveOnlyReason must be a string`);
continue;
}
const fast = entry.fast ?? [];
const liveOnlyReason = entry.liveOnlyReason?.trim() ?? "";
if (fast.length > 0 && liveOnlyReason) {
errors.push(`${entry.live}: choose fast tests or a live-only reason, not both`);
} else if (fast.length === 0 && !liveOnlyReason) {
errors.push(`${entry.live}: map at least one fast test or provide a live-only reason`);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if (!fileExists(entry.live)) errors.push(`${entry.live}: live test does not exist`);
for (const fastFile of new Set(fast)) {
if (!isFastPrTest(fastFile)) {
errors.push(`${entry.live}: ${fastFile} is not collected by a fast PR test project`);
} else if (!fileExists(fastFile)) {
errors.push(`${entry.live}: mapped fast test does not exist: ${fastFile}`);
}
}
}

for (const liveFile of [...new Set(changedFiles)].filter((file) => LIVE_TEST.test(file))) {
if (!entries.has(liveFile)) {
errors.push(`${liveFile}: changed live E2E needs an entry in ${DEFAULT_PARITY_MANIFEST}`);
}
}

return errors.sort();
}

function argument(name: string): string | undefined {
const index = process.argv.indexOf(name);
return index >= 0 ? process.argv[index + 1] : undefined;
}

function changedFiles(base: string, head: string): string[] {
return execFileSync("git", ["diff", "--name-only", "--diff-filter=ACMR", `${base}...${head}`], {
cwd: REPO_ROOT,
encoding: "utf8",
})
.split(/\r?\n/u)
.filter(Boolean);
}

function main(): void {
const base = argument("--base");
const head = argument("--head") ?? "HEAD";
if (!base) throw new Error("usage: e2e-mock-parity.ts --base <git-ref> [--head <git-ref>]");

const manifestPath = path.join(REPO_ROOT, DEFAULT_PARITY_MANIFEST);
const manifest = JSON.parse(fs.readFileSync(manifestPath, "utf8")) as MockParityManifest;
const errors = validateMockParity({ manifest, changedFiles: changedFiles(base, head) });
if (errors.length > 0) {
console.error(
["E2E mock/live parity check failed:", ...errors.map((error) => `- ${error}`)].join("\n"),
);
process.exitCode = 1;
return;
}
console.log("E2E mock/live parity check passed.");
}

if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) main();
Loading