diff --git a/.github/workflows/nightly-e2e.yaml b/.github/workflows/nightly-e2e.yaml index 280daa0b52..20e2c0e097 100644 --- a/.github/workflows/nightly-e2e.yaml +++ b/.github/workflows/nightly-e2e.yaml @@ -1716,15 +1716,23 @@ jobs: - *target-ref-checkout - *dockerhub-auth-step - - name: Install NemoClaw - env: - NVIDIA_INFERENCE_API_KEY: ${{ secrets.NVIDIA_INFERENCE_API_KEY }} - NEMOCLAW_NON_INTERACTIVE: "1" - NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" - run: bash install.sh --non-interactive --yes-i-accept-third-party-software + - name: Set up Node + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.0.0 + with: + node-version: 22 + cache: npm + + - name: Install root dependencies + run: npm ci --ignore-scripts + + - name: Build CLI + run: npm run build:cli + + - name: Install OpenShell CLI + run: env -u DOCKER_CONFIG -u DOCKERHUB_USERNAME -u DOCKERHUB_TOKEN -u NVIDIA_INFERENCE_API_KEY -u GITHUB_TOKEN bash scripts/install-openshell.sh + - name: Run double onboard E2E test env: - NVIDIA_INFERENCE_API_KEY: ${{ secrets.NVIDIA_INFERENCE_API_KEY }} NEMOCLAW_NON_INTERACTIVE: "1" NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" run: | @@ -2256,15 +2264,22 @@ jobs: steps: - *target-ref-checkout - *dockerhub-auth-step - - name: Install NemoClaw - env: - NVIDIA_INFERENCE_API_KEY: ${{ secrets.NVIDIA_INFERENCE_API_KEY }} - NEMOCLAW_NON_INTERACTIVE: "1" - NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" - run: bash install.sh --non-interactive --yes-i-accept-third-party-software + - name: Set up Node + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.0.0 + with: + node-version: 22 + cache: npm + + - name: Install root dependencies + run: npm ci --ignore-scripts + + - name: Build CLI + run: npm run build:cli + + - name: Install OpenShell CLI + run: env -u DOCKER_CONFIG -u DOCKERHUB_USERNAME -u DOCKERHUB_TOKEN -u NVIDIA_INFERENCE_API_KEY -u GITHUB_TOKEN bash scripts/install-openshell.sh + - name: Run concurrent gateway ports E2E test - env: - NVIDIA_INFERENCE_API_KEY: ${{ secrets.NVIDIA_INFERENCE_API_KEY }} run: | [ -f "$HOME/.bashrc" ] && source "$HOME/.bashrc" 2>/dev/null || true export NVM_DIR="${NVM_DIR:-$HOME/.nvm}" diff --git a/biome.json b/biome.json index 8e7c2b3d29..1fc05b56a2 100644 --- a/biome.json +++ b/biome.json @@ -16,6 +16,7 @@ "nemoclaw/package-lock.json", "*.ts", "**/*.ts", + "**/*.mts", "bin/**/*.js", "scripts/**/*.js", "scripts/**/*.mjs", diff --git a/scripts/generate-openclaw-config.mts b/scripts/generate-openclaw-config.mts index 59f8c86649..a01c71cea4 100755 --- a/scripts/generate-openclaw-config.mts +++ b/scripts/generate-openclaw-config.mts @@ -138,7 +138,9 @@ function buildOpenClawOtelConfig(env: Env): JsonObject | undefined { throw new Error("NEMOCLAW_OPENCLAW_OTEL_ENDPOINT must not include credentials"); } - const serviceName = (env.NEMOCLAW_OPENCLAW_OTEL_SERVICE_NAME || DEFAULT_OPENCLAW_OTEL_SERVICE_NAME).trim(); + const serviceName = ( + env.NEMOCLAW_OPENCLAW_OTEL_SERVICE_NAME || DEFAULT_OPENCLAW_OTEL_SERVICE_NAME + ).trim(); if (!serviceName) { throw new Error("NEMOCLAW_OPENCLAW_OTEL_SERVICE_NAME must not be empty"); } @@ -596,9 +598,7 @@ function validateExtraAgentTools(entry: JsonObject, label: string): JsonObject { const value = tools[key]; if (value === undefined) continue; if (!Array.isArray(value) || value.some((token) => typeof token !== "string" || !token)) { - throw new Error( - `${label}.tools.${key} must be an array of non-empty strings when present.`, - ); + throw new Error(`${label}.tools.${key} must be an array of non-empty strings when present.`); } } if (tools.profile !== undefined && typeof tools.profile !== "string") { @@ -617,9 +617,7 @@ function validateExtraAgentSubagents(entry: JsonObject, label: string): JsonObje rejectUnknownKeys(subagents, ALLOWED_SUBAGENTS_KEYS, `${label}.subagents`); const depth = subagents.maxSpawnDepth; if (typeof depth !== "number" || !Number.isInteger(depth) || depth < 0) { - throw new Error( - `${label}.subagents.maxSpawnDepth must be a non-negative integer.`, - ); + throw new Error(`${label}.subagents.maxSpawnDepth must be a non-negative integer.`); } return pickAllowed(subagents, ALLOWED_SUBAGENTS_KEYS); } @@ -629,9 +627,7 @@ function validateExtraAgents(value: unknown): JsonObject[] { return []; } if (!Array.isArray(value)) { - throw new Error( - "NEMOCLAW_EXTRA_AGENTS_JSON must decode to a JSON array of agent objects", - ); + throw new Error("NEMOCLAW_EXTRA_AGENTS_JSON must decode to a JSON array of agent objects"); } const seenIds = new Set([MAIN_AGENT_ID]); return value.map((entry, index) => { @@ -661,9 +657,7 @@ function validateExtraAgents(value: unknown): JsonObject[] { throw new Error(`${label}.${pathKey} must be a non-empty string`); } if (!isAbsolute(pathValue)) { - throw new Error( - `${label}.${pathKey} must be an absolute path, got "${pathValue}"`, - ); + throw new Error(`${label}.${pathKey} must be an absolute path, got "${pathValue}"`); } const expected = expectedAgentPath(pathKey, id); if (resolve(pathValue) !== expected) { @@ -830,7 +824,6 @@ export function buildConfig(env: Env = process.env): JsonObject { inferenceCompat.supportsUsageInStreaming ??= true; } - const normalizedUrl = normalizeUrlForParse(chatUiUrl); const parsed = parseUrl(normalizedUrl); const loopbackOrigin = `http://127.0.0.1:${gatewayPort}`; @@ -964,7 +957,6 @@ export function buildConfig(env: Env = process.env): JsonObject { }; } - return config; } diff --git a/scripts/patch-openclaw-slack-deny-feedback.mts b/scripts/patch-openclaw-slack-deny-feedback.mts index bfedc7f5da..edca0b9e6d 100755 --- a/scripts/patch-openclaw-slack-deny-feedback.mts +++ b/scripts/patch-openclaw-slack-deny-feedback.mts @@ -149,7 +149,7 @@ function buildHelperSource(): string { "\t\t// Only fall back to a DM when Slack definitively did not deliver the", "\t\t// ephemeral. Ambiguous failures (network/HTTP, timeout, service errors)", "\t\t// may have been accepted, so a DM there could double-notify the sender.", - '\t\tconst ephemeralErrorCode = ephemeralError?.data?.error ?? ephemeralError?.code;', + "\t\tconst ephemeralErrorCode = ephemeralError?.data?.error ?? ephemeralError?.code;", '\t\tctx?.logger?.warn?.({ err: ephemeralError, channel, code: ephemeralErrorCode }, "nemoclaw: slack denial ephemeral feedback failed (#4752)");', '\t\tconst nonDeliveryCodes = ["user_not_in_channel", "not_in_channel", "channel_not_found", "cannot_reply_to_message", "is_archived", "messages_tab_disabled"];', "\t\tif (!nonDeliveryCodes.includes(ephemeralErrorCode)) return;", diff --git a/src/lib/messaging/applier/build/messaging-build-applier.mts b/src/lib/messaging/applier/build/messaging-build-applier.mts index fe9968c9a7..36cf5578f6 100755 --- a/src/lib/messaging/applier/build/messaging-build-applier.mts +++ b/src/lib/messaging/applier/build/messaging-build-applier.mts @@ -3,13 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 import { spawnSync } from "node:child_process"; -import { - chmodSync, - existsSync, - mkdirSync, - readFileSync, - writeFileSync, -} from "node:fs"; +import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join, resolve, sep } from "node:path"; import { pathToFileURL } from "node:url"; @@ -207,12 +201,16 @@ export function applyMessagingAgentRenderToLocalFiles( for (const [target, renderEntries] of grouped) { const kinds = uniqueStrings(renderEntries.map((entry) => entry.kind)); if (kinds.length !== 1) { - throw new MessagingBuildApplierError(`Cannot apply mixed messaging render kinds to ${target}.`); + throw new MessagingBuildApplierError( + `Cannot apply mixed messaging render kinds to ${target}.`, + ); } if (kinds[0] === "json-fragment") { appliedTargets.push(applyJsonRenderEntriesToLocalFile(plan, target, renderEntries, options)); } else { - appliedTargets.push(applyEnvRenderEntriesToLocalFile(plan.agent, target, renderEntries, options)); + appliedTargets.push( + applyEnvRenderEntriesToLocalFile(plan.agent, target, renderEntries, options), + ); } } @@ -224,7 +222,9 @@ export function activeChannels(plan: MessagingBuildPlan | null): string[] { const seen = new Set(); const channels: string[] = []; for (const item of plan.channels) { - const channel = String(item.channelId || "").trim().toLowerCase(); + const channel = String(item.channelId || "") + .trim() + .toLowerCase(); if (!channel || seen.has(channel)) continue; if (item.active === true && item.disabled !== true) { seen.add(channel); @@ -276,10 +276,7 @@ export function openClawDoctorEnvOverrides( return overrides; } -export function installOpenClawMessagingPlugins( - plan: MessagingBuildPlan | null, - env: Env, -): void { +export function installOpenClawMessagingPlugins(plan: MessagingBuildPlan | null, env: Env): void { for (const spec of collectOpenClawMessagingPluginInstallSpecs(plan, env)) { runCommand(["openclaw", "plugins", "install", spec, "--pin"], env); } @@ -338,7 +335,9 @@ function applyJsonRenderEntriesToLocalFile( mkdirSync(dirname(targetPath), { recursive: true }); writeFileSync( targetPath, - targetPath.endsWith(".yaml") ? serializeGeneratedYamlObject(config) : `${JSON.stringify(config, null, 2)}\n`, + targetPath.endsWith(".yaml") + ? serializeGeneratedYamlObject(config) + : `${JSON.stringify(config, null, 2)}\n`, ); chmodSync(targetPath, 0o600); return targetPath; @@ -351,7 +350,10 @@ function applyEnvRenderEntriesToLocalFile( options: { readonly homeDir?: string }, ): string { const targetPath = resolveAgentRenderTarget(agent, target, options); - const envLines = readTextIfExists(targetPath)?.split(/\r?\n/).filter((line) => line.length > 0) ?? []; + const envLines = + readTextIfExists(targetPath) + ?.split(/\r?\n/) + .filter((line) => line.length > 0) ?? []; for (const render of renderEntries) { if (!Array.isArray(render.lines)) { throw new MessagingBuildApplierError( @@ -375,7 +377,9 @@ function applyMessagingRenderEntriesToObject( const rules = credentialPlaceholderRules(plan); for (const render of renderEntries) { if (render.kind !== "json-fragment" || typeof render.path !== "string") { - throw new MessagingBuildApplierError(`Messaging render for ${target} must be a JSON fragment with a path.`); + throw new MessagingBuildApplierError( + `Messaging render for ${target} must be a JSON fragment with a path.`, + ); } const value = preserveCredentialPlaceholders( requiredSerializableValue(render.value, "render value"), @@ -395,7 +399,9 @@ function readEnvRenderLines(render: MessagingRenderEntry): readonly string[] { for (const line of render.lines) { if (/[\r\n]/.test(line)) { throw new MessagingBuildApplierError( - "Messaging env render '" + (render.renderId ?? render.channelId) + "' must not contain line breaks.", + "Messaging env render '" + + (render.renderId ?? render.channelId) + + "' must not contain line breaks.", ); } } @@ -432,19 +438,26 @@ function resolveAgentRenderTarget( let relativePath: string | null = null; if (target.startsWith("~/.openclaw/")) { if (agent !== "openclaw") { - throw new MessagingBuildApplierError(`Messaging render target ${target} does not match ${agent}.`); + throw new MessagingBuildApplierError( + `Messaging render target ${target} does not match ${agent}.`, + ); } relativePath = target.slice("~/.openclaw/".length); } if (target.startsWith("~/.hermes/")) { if (agent !== "hermes") { - throw new MessagingBuildApplierError(`Messaging render target ${target} does not match ${agent}.`); + throw new MessagingBuildApplierError( + `Messaging render target ${target} does not match ${agent}.`, + ); } relativePath = target.slice("~/.hermes/".length); } if (relativePath !== null) { const resolvedTarget = resolve(agentRoot, relativePath); - if (resolvedTarget !== normalizedRoot && !resolvedTarget.startsWith(`${normalizedRoot}${sep}`)) { + if ( + resolvedTarget !== normalizedRoot && + !resolvedTarget.startsWith(`${normalizedRoot}${sep}`) + ) { throw new MessagingBuildApplierError( `Messaging render target ${target} must stay inside ${agentRoot}.`, ); @@ -504,9 +517,10 @@ function applyBuildFileOutputToLocalAgentRoot( file: BuildFileOutput, options: { readonly homeDir?: string } = {}, ): string { - const root = agent === "hermes" - ? join(options.homeDir ?? homedir(), ".hermes") - : join(options.homeDir ?? homedir(), ".openclaw"); + const root = + agent === "hermes" + ? join(options.homeDir ?? homedir(), ".hermes") + : join(options.homeDir ?? homedir(), ".openclaw"); const relativePath = normalizeBuildFilePath(file.path); const target = resolve(root, relativePath); const normalizedRoot = resolve(root); @@ -532,7 +546,9 @@ function mergeBuildFileContent( target: string, ): string { if (!isObject(patch)) { - throw new MessagingBuildApplierError(`Messaging build-file merge for ${target} must be an object.`); + throw new MessagingBuildApplierError( + `Messaging build-file merge for ${target} must be an object.`, + ); } const root = parseJsonObject(existing, target); mergeJsonObjects(root, patch as JsonObject); @@ -543,7 +559,9 @@ function parseJsonObject(existing: string | undefined, target: string): JsonObje if (!existing || existing.trim().length === 0) return {}; const parsed = JSON.parse(existing) as unknown; if (!isObject(parsed)) { - throw new MessagingBuildApplierError(`Messaging build-file target ${target} must contain an object.`); + throw new MessagingBuildApplierError( + `Messaging build-file target ${target} must contain an object.`, + ); } return parsed as JsonObject; } @@ -561,7 +579,9 @@ function readBuildFileOutput(value: MessagingSerializableValue): BuildFileOutput throw new MessagingBuildApplierError("Messaging build-file output must include a path"); } if (file.content === undefined && file.merge === undefined) { - throw new MessagingBuildApplierError(`Messaging build-file ${file.path} must include content or merge`); + throw new MessagingBuildApplierError( + `Messaging build-file ${file.path} must include content or merge`, + ); } if (file.mode !== undefined && typeof file.mode !== "string") { throw new MessagingBuildApplierError(`Messaging build-file ${file.path} mode must be a string`); @@ -571,11 +591,15 @@ function readBuildFileOutput(value: MessagingSerializableValue): BuildFileOutput function normalizeBuildFilePath(pathValue: string): string { if (pathValue.startsWith("/") || pathValue.includes("\\") || /[\0-\x1F\x7F]/.test(pathValue)) { - throw new MessagingBuildApplierError(`Messaging build-file path ${pathValue} must be a safe relative path`); + throw new MessagingBuildApplierError( + `Messaging build-file path ${pathValue} must be a safe relative path`, + ); } const segments = pathValue.split("/"); if (segments.some((segment) => !segment || segment === "." || segment === "..")) { - throw new MessagingBuildApplierError(`Messaging build-file path ${pathValue} must not traverse directories`); + throw new MessagingBuildApplierError( + `Messaging build-file path ${pathValue} must not traverse directories`, + ); } return pathValue; } @@ -588,11 +612,15 @@ function serializeBuildFileContent(value: MessagingSerializableValue | undefined function parseBuildFileMode(pathValue: string, mode: string): number { if (!/^[0-7]{3,4}$/.test(mode) || (mode.length === 4 && mode[0] !== "0")) { - throw new MessagingBuildApplierError(`Messaging build-file ${pathValue} mode must be an octal file mode`); + throw new MessagingBuildApplierError( + `Messaging build-file ${pathValue} mode must be an octal file mode`, + ); } const parsed = Number.parseInt(mode, 8); if ((parsed & 0o022) !== 0) { - throw new MessagingBuildApplierError(`Messaging build-file ${pathValue} mode must not be group/world writable`); + throw new MessagingBuildApplierError( + `Messaging build-file ${pathValue} mode must not be group/world writable`, + ); } return parsed; } @@ -795,7 +823,9 @@ function setJsonPath(root: JsonObject, pathValue: string, value: MessagingSerial function mergeJsonObjects(target: JsonObject, patch: JsonObject): void { for (const [key, value] of Object.entries(patch)) { if (key === "__proto__" || key === "prototype" || key === "constructor") { - throw new MessagingBuildApplierError("Messaging object merge rejected unsafe object key " + key); + throw new MessagingBuildApplierError( + "Messaging object merge rejected unsafe object key " + key, + ); } const existing = target[key]; if (isObject(existing) && isObject(value)) { @@ -971,7 +1001,11 @@ function parseGeneratedYamlArray( return [parsed, index]; } -function parseGeneratedYamlScalar(value: string, target: string, lineNumber: number): MessagingSerializableValue { +function parseGeneratedYamlScalar( + value: string, + target: string, + lineNumber: number, +): MessagingSerializableValue { if (value === "[]") return []; if (value === "{}") return {}; if (value === "null") return null; @@ -994,7 +1028,10 @@ function serializeGeneratedYamlObject(value: JsonObject): string { return serializeGeneratedYamlValue(value); } -function serializeGeneratedYamlValue(value: MessagingSerializableValue, indent: number = 0): string { +function serializeGeneratedYamlValue( + value: MessagingSerializableValue, + indent: number = 0, +): string { const pad = " ".repeat(indent); if (Array.isArray(value)) { if (value.length === 0) return `${pad}[]\n`; @@ -1017,10 +1054,16 @@ function serializeGeneratedYamlValue(value: MessagingSerializableValue, indent: for (const [key, item] of Object.entries(value)) { assertSafeObjectKey(key, "Messaging YAML object"); if (Array.isArray(item)) { - out += item.length === 0 ? `${pad}${key}: []\n` : `${pad}${key}:\n${serializeGeneratedYamlValue(item, indent + 1)}`; + out += + item.length === 0 + ? `${pad}${key}: []\n` + : `${pad}${key}:\n${serializeGeneratedYamlValue(item, indent + 1)}`; } else if (isObject(item)) { const entries = Object.entries(item); - out += entries.length === 0 ? `${pad}${key}: {}\n` : `${pad}${key}:\n${serializeGeneratedYamlValue(item as MessagingSerializableValue, indent + 1)}`; + out += + entries.length === 0 + ? `${pad}${key}: {}\n` + : `${pad}${key}:\n${serializeGeneratedYamlValue(item as MessagingSerializableValue, indent + 1)}`; } else { out += `${pad}${key}: ${formatGeneratedYamlScalar(item as MessagingSerializableValue)}\n`; } @@ -1122,13 +1165,17 @@ export function describeMessagingBuildPhase( plan: MessagingBuildPlan | null, phase: MessagingBuildPhase, env: Env, -): BuildCommandResult & { readonly agent: MessagingAgentId | "unknown"; readonly phase: MessagingBuildPhase } { +): BuildCommandResult & { + readonly agent: MessagingAgentId | "unknown"; + readonly phase: MessagingBuildPhase; +} { return { agent: plan?.agent ?? "unknown", phase, channels: activeChannels(plan), doctorEnv: plan?.agent === "openclaw" ? openClawDoctorEnvOverrides(plan, env) : {}, - installSpecs: plan?.agent === "openclaw" ? collectOpenClawMessagingPluginInstallSpecs(plan, env) : [], + installSpecs: + plan?.agent === "openclaw" ? collectOpenClawMessagingPluginInstallSpecs(plan, env) : [], openclawVersion: env.OPENCLAW_VERSION || "", }; } diff --git a/test/e2e-scenario/fixtures/fake-openai-compatible.ts b/test/e2e-scenario/fixtures/fake-openai-compatible.ts new file mode 100644 index 0000000000..74179ca70b --- /dev/null +++ b/test/e2e-scenario/fixtures/fake-openai-compatible.ts @@ -0,0 +1,160 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { type ChildProcess, spawn } from "node:child_process"; +import fs from "node:fs"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; + +const REPO_ROOT = path.resolve(import.meta.dirname, "../../.."); +const SERVER_SCRIPT = path.join(REPO_ROOT, "test/e2e/lib/fake-openai-compatible-api.mts"); + +export interface FakeOpenAiCompatibleRequest { + readonly method: string; + readonly path: string; + readonly bodyBytes: number; + readonly auth?: string; + readonly model?: string; + readonly stream?: boolean; +} + +export interface FakeOpenAiCompatibleServer { + readonly baseUrl: string; + readonly logFile: string; + readonly requestsFile: string; + requests(): readonly FakeOpenAiCompatibleRequest[]; + close(): Promise; +} + +export interface FakeOpenAiCompatibleServerOptions { + readonly apiKey?: string; + readonly chatContent?: string; + readonly host?: string; + readonly model?: string; + readonly port?: number; + readonly publicHost?: string; + readonly requireAuth?: boolean; + readonly responseText?: string; +} + +function readPort(portFile: string): number | null { + try { + const value = Number(fs.readFileSync(portFile, "utf8").trim()); + return Number.isInteger(value) && value > 0 ? value : null; + } catch { + return null; + } +} + +function waitForExit(child: ChildProcess): Promise { + if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve(); + return new Promise((resolve) => { + child.once("exit", () => resolve()); + }); +} + +function readinessProbeHost(host: string): string { + if (host === "0.0.0.0") return "127.0.0.1"; + if (host === "::") return "::1"; + return host; +} + +function formatHttpHost(host: string): string { + return host.includes(":") && !host.startsWith("[") ? `[${host}]` : host; +} + +function canReachModels(host: string, port: number): Promise { + return new Promise((resolve) => { + const req = http.get( + { + host: readinessProbeHost(host), + path: "/v1/models", + port, + timeout: 1_000, + }, + (res) => { + res.resume(); + resolve(res.statusCode === 200); + }, + ); + req.on("error", () => resolve(false)); + req.on("timeout", () => { + req.destroy(); + resolve(false); + }); + }); +} + +async function waitForReady(portFile: string, child: ChildProcess, host: string): Promise { + const deadline = Date.now() + 30_000; + while (Date.now() < deadline) { + if (child.exitCode !== null || child.signalCode !== null) { + throw new Error("fake OpenAI-compatible endpoint exited before becoming ready"); + } + const port = readPort(portFile); + if (port !== null && (await canReachModels(host, port))) return port; + await new Promise((resolve) => setTimeout(resolve, 250)); + } + throw new Error("fake OpenAI-compatible endpoint did not become ready"); +} + +function parseRequests(requestsFile: string): FakeOpenAiCompatibleRequest[] { + try { + return fs + .readFileSync(requestsFile, "utf8") + .split("\n") + .filter(Boolean) + .map((line) => JSON.parse(line) as FakeOpenAiCompatibleRequest); + } catch { + return []; + } +} + +export async function startFakeOpenAiCompatibleServer( + options: FakeOpenAiCompatibleServerOptions = {}, +): Promise { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-fake-openai-")); + const portFile = path.join(tmpDir, "port"); + const logFile = path.join(tmpDir, "server.log"); + const requestsFile = path.join(tmpDir, "requests.jsonl"); + const host = options.host ?? "127.0.0.1"; + const child = spawn(process.execPath, ["--experimental-strip-types", SERVER_SCRIPT], { + env: { + ...process.env, + NEMOCLAW_FAKE_OPENAI_API_KEY: options.apiKey ?? "", + NEMOCLAW_FAKE_OPENAI_CHAT_CONTENT: options.chatContent ?? "ok", + NEMOCLAW_FAKE_OPENAI_HOST: host, + NEMOCLAW_FAKE_OPENAI_LOG_FILE: logFile, + NEMOCLAW_FAKE_OPENAI_MODEL: options.model ?? "test-model", + NEMOCLAW_FAKE_OPENAI_PORT: String(options.port ?? 0), + NEMOCLAW_FAKE_OPENAI_PORT_FILE: portFile, + NEMOCLAW_FAKE_OPENAI_REQUESTS_FILE: requestsFile, + NEMOCLAW_FAKE_OPENAI_REQUIRE_AUTH: options.requireAuth ? "1" : "0", + NEMOCLAW_FAKE_OPENAI_RESPONSE_TEXT: options.responseText ?? options.chatContent ?? "ok", + }, + stdio: "ignore", + }); + + let port: number; + try { + port = await waitForReady(portFile, child, host); + } catch (error) { + child.kill("SIGTERM"); + await waitForExit(child); + fs.rmSync(tmpDir, { force: true, recursive: true }); + throw error; + } + const publicHost = options.publicHost ?? readinessProbeHost(host); + return { + baseUrl: `http://${formatHttpHost(publicHost)}:${port}/v1`, + logFile, + requestsFile, + requests: () => parseRequests(requestsFile), + close: async () => { + child.kill("SIGTERM"); + await waitForExit(child); + fs.rmSync(tmpDir, { force: true, recursive: true }); + }, + }; +} diff --git a/test/e2e-scenario/live/double-onboard.test.ts b/test/e2e-scenario/live/double-onboard.test.ts index d3886b3082..c7e34ff15f 100644 --- a/test/e2e-scenario/live/double-onboard.test.ts +++ b/test/e2e-scenario/live/double-onboard.test.ts @@ -2,16 +2,14 @@ // SPDX-License-Identifier: Apache-2.0 import fs from "node:fs"; -import http from "node:http"; -import type { AddressInfo } from "node:net"; import os from "node:os"; import path from "node:path"; -import type { ArtifactSink } from "../fixtures/artifacts.ts"; import { buildAvailabilityProbeEnv } from "../fixtures/availability-env.ts"; import type { HostCliClient } from "../fixtures/clients/host.ts"; import type { SandboxClient } from "../fixtures/clients/sandbox.ts"; import { validateSandboxName } from "../fixtures/clients/sandbox.ts"; import { expect, test } from "../fixtures/e2e-test.ts"; +import { startFakeOpenAiCompatibleServer } from "../fixtures/fake-openai-compatible.ts"; import { shouldRunLiveE2EScenarios } from "../fixtures/live-project-gate.ts"; import type { ShellProbeResult } from "../fixtures/shell-probe.ts"; @@ -46,12 +44,6 @@ validateSandboxName(SANDBOX_B); if (INSTALL_SANDBOX_NAME) validateSandboxName(INSTALL_SANDBOX_NAME); validateSandboxName(ALT_GATEWAY_NAME); -interface FakeOpenAiServer { - baseUrl: string; - close: () => Promise; - requests: () => readonly string[]; -} - function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } @@ -201,77 +193,6 @@ async function cleanupDoubleOnboardState( ); } -async function startFakeOpenAi(artifacts: ArtifactSink): Promise { - const requests: string[] = []; - const server = http.createServer((req, res) => { - requests.push(`${req.method ?? "GET"} ${req.url ?? "/"}`); - req.resume(); - const send = (status: number, payload: unknown) => { - const body = JSON.stringify(payload); - res.writeHead(status, { - "content-type": "application/json", - "content-length": Buffer.byteLength(body), - }); - res.end(body); - }; - if (req.method === "GET" && (req.url === "/v1/models" || req.url === "/models")) { - send(200, { data: [{ id: "test-model", object: "model" }] }); - return; - } - if ( - req.method === "POST" && - (req.url === "/v1/chat/completions" || req.url === "/chat/completions") - ) { - send(200, { - id: "chatcmpl-test", - object: "chat.completion", - choices: [ - { - index: 0, - message: { role: "assistant", content: "ok" }, - finish_reason: "stop", - }, - ], - }); - return; - } - if (req.method === "POST" && (req.url === "/v1/responses" || req.url === "/responses")) { - send(200, { - id: "resp-test", - object: "response", - output: [ - { - type: "message", - role: "assistant", - content: [{ type: "output_text", text: "ok" }], - }, - ], - }); - return; - } - send(404, { error: { message: "not found" } }); - }); - - const requestedPort = Number(process.env.NEMOCLAW_FAKE_PORT ?? 0); - await new Promise((resolve, reject) => { - server.once("error", reject); - server.listen(requestedPort, "127.0.0.1", resolve); - }); - const address = server.address() as AddressInfo; - const baseUrl = `http://127.0.0.1:${address.port}/v1`; - await artifacts.writeJson("fake-openai.json", { baseUrl }); - return { - baseUrl, - requests: () => requests, - close: async () => { - await artifacts.writeJson("fake-openai-requests.json", requests); - await new Promise((resolve, reject) => { - server.close((error) => (error ? reject(error) : resolve())); - }); - }, - }; -} - async function gatewayRuntimeId(host: HostCliClient, artifactName: string): Promise { const script = String.raw` set -euo pipefail @@ -502,8 +423,12 @@ liveTest( "prereq-nemoclaw", ); - const fake = await startFakeOpenAi(artifacts); + const fake = await startFakeOpenAiCompatibleServer({ + port: Number(process.env.NEMOCLAW_FAKE_PORT ?? 0), + }); + await artifacts.writeJson("fake-openai.json", { baseUrl: fake.baseUrl }); cleanup.add("close fake OpenAI-compatible endpoint", async () => { + await artifacts.writeJson("fake-openai-requests.json", fake.requests()); await fake.close(); }); cleanup.add("remove double-onboard sandboxes and gateways", async () => { diff --git a/test/e2e-scenario/live/token-rotation.test.ts b/test/e2e-scenario/live/token-rotation.test.ts index 40a938082f..2b1d77bd1f 100644 --- a/test/e2e-scenario/live/token-rotation.test.ts +++ b/test/e2e-scenario/live/token-rotation.test.ts @@ -2,15 +2,13 @@ // SPDX-License-Identifier: Apache-2.0 import fs from "node:fs"; -import http from "node:http"; -import type { AddressInfo } from "node:net"; import path from "node:path"; - +import { testTimeoutOptions } from "../../helpers/timeouts"; import { buildAvailabilityProbeEnv } from "../fixtures/availability-env.ts"; import { validateSandboxName } from "../fixtures/clients/sandbox.ts"; import { expect, test } from "../fixtures/e2e-test.ts"; +import { startFakeOpenAiCompatibleServer } from "../fixtures/fake-openai-compatible.ts"; import { shouldRunLiveE2EScenarios } from "../fixtures/live-project-gate.ts"; -import { testTimeoutOptions } from "../../helpers/timeouts"; // Focused Vitest replacement coverage for test/e2e/test-token-rotation.sh. // Keep this free-standing and direct: the legacy contract is the real CLI + @@ -54,18 +52,6 @@ type RegistryCredentialBinding = { credentialHash?: unknown; }; -type FakeOpenAIRequest = { - method: string; - path: string; - bodyBytes: number; -}; - -type FakeOpenAIEndpoint = { - baseUrl: string; - requests: FakeOpenAIRequest[]; - close(): Promise; -}; - type RegistrySandboxEntry = { messaging?: { plan?: { @@ -82,92 +68,6 @@ function stripAnsi(value: string): string { return value.replace(/\u001B\[[0-?]*[ -/]*[@-~]/g, ""); } -function jsonResponse(res: http.ServerResponse, status: number, payload: unknown): void { - const body = JSON.stringify(payload); - res.writeHead(status, { - "Content-Type": "application/json", - "Content-Length": Buffer.byteLength(body), - }); - res.end(body); -} - -async function startFakeOpenAIEndpoint(): Promise { - const requests: FakeOpenAIRequest[] = []; - const server = http.createServer((req, res) => { - let bodyBytes = 0; - req.on("data", (chunk: Buffer) => { - bodyBytes += chunk.length; - }); - req.on("end", () => { - const requestPath = new URL(req.url ?? "/", "http://fake.local").pathname; - requests.push({ - method: req.method ?? "GET", - path: requestPath, - bodyBytes, - }); - if (req.method === "GET" && ["/v1/models", "/models"].includes(requestPath)) { - jsonResponse(res, 200, { - data: [{ id: "test-model", object: "model" }], - }); - return; - } - if ( - req.method === "POST" && - ["/v1/chat/completions", "/chat/completions"].includes(requestPath) - ) { - jsonResponse(res, 200, { - id: "chatcmpl-token-rotation-e2e", - object: "chat.completion", - choices: [ - { - index: 0, - message: { role: "assistant", content: "OK" }, - finish_reason: "stop", - }, - ], - }); - return; - } - if (req.method === "POST" && ["/v1/responses", "/responses"].includes(requestPath)) { - jsonResponse(res, 200, { - id: "resp-token-rotation-e2e", - object: "response", - output: [ - { - type: "message", - role: "assistant", - content: [{ type: "output_text", text: "OK" }], - }, - ], - }); - return; - } - jsonResponse(res, 404, { error: { message: "not found" } }); - }); - }); - - await new Promise((resolve, reject) => { - server.once("error", reject); - server.listen(0, "0.0.0.0", () => { - server.off("error", reject); - resolve(); - }); - }); - const address = server.address(); - if (!address || typeof address === "string") { - throw new Error("fake OpenAI-compatible endpoint did not bind to a TCP port"); - } - const port = (address as AddressInfo).port; - return { - baseUrl: `http://host.openshell.internal:${port}/v1`, - requests, - close: () => - new Promise((resolve, reject) => { - server.close((error) => (error ? reject(error) : resolve())); - }), - }; -} - function onboardEnv(endpointUrl: string, tokens: TokenSet): NodeJS.ProcessEnv { return { ...buildAvailabilityProbeEnv(), @@ -381,9 +281,14 @@ liveTest( skip("Docker is required for token rotation live E2E"); } - const fakeOpenAI = await startFakeOpenAIEndpoint(); + const fakeOpenAI = await startFakeOpenAiCompatibleServer({ + chatContent: "OK", + host: "0.0.0.0", + publicHost: "host.openshell.internal", + responseText: "OK", + }); cleanup.add("stop fake OpenAI-compatible endpoint for token rotation", async () => { - await artifacts.writeJson("fake-openai-compatible-requests.json", fakeOpenAI.requests); + await artifacts.writeJson("fake-openai-compatible-requests.json", fakeOpenAI.requests()); await fakeOpenAI.close(); }); diff --git a/test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts b/test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts index 5cddff8838..db85408c30 100644 --- a/test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts +++ b/test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts @@ -464,7 +464,7 @@ jobs: } }); - it("keeps each free-standing scenario out of the registry matrix", { timeout: 60_000 }, () => { + it("keeps each free-standing scenario out of the registry matrix", { timeout: 120_000 }, () => { const inventory = readFreeStandingJobsInventory(); for (const job of inventory.allowedJobs) { expect(generateMatrixForDispatch({ JOBS: job, SCENARIOS: "" })).toMatchObject({ diff --git a/test/e2e/lib/fake-openai-compatible-api.mts b/test/e2e/lib/fake-openai-compatible-api.mts new file mode 100755 index 0000000000..1aaa8f156e --- /dev/null +++ b/test/e2e/lib/fake-openai-compatible-api.mts @@ -0,0 +1,194 @@ +#!/usr/bin/env -S node --experimental-strip-types +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { appendFileSync, writeFileSync } from "node:fs"; +import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; + +type JsonObject = Record; + +const host = process.env.NEMOCLAW_FAKE_OPENAI_HOST || "127.0.0.1"; +const port = Number(process.env.NEMOCLAW_FAKE_OPENAI_PORT || "0"); +const portFile = process.env.NEMOCLAW_FAKE_OPENAI_PORT_FILE || ""; +const logFile = process.env.NEMOCLAW_FAKE_OPENAI_LOG_FILE || ""; +const requestsFile = process.env.NEMOCLAW_FAKE_OPENAI_REQUESTS_FILE || ""; +const model = process.env.NEMOCLAW_FAKE_OPENAI_MODEL || "test-model"; +const apiKey = process.env.NEMOCLAW_FAKE_OPENAI_API_KEY || ""; +const requireAuth = process.env.NEMOCLAW_FAKE_OPENAI_REQUIRE_AUTH === "1"; +const chatContent = process.env.NEMOCLAW_FAKE_OPENAI_CHAT_CONTENT || "ok"; +const responseText = process.env.NEMOCLAW_FAKE_OPENAI_RESPONSE_TEXT || chatContent; + +function log(message: string): void { + if (logFile) { + appendFileSync(logFile, `${message}\n`); + return; + } + console.log(message); +} + +function recordRequest(entry: JsonObject): void { + if (!requestsFile) return; + appendFileSync(requestsFile, `${JSON.stringify(entry)}\n`); +} + +function sendJson(res: ServerResponse, status: number, payload: unknown): void { + const body = JSON.stringify(payload); + res.writeHead(status, { + "Content-Type": "application/json", + "Content-Length": Buffer.byteLength(body), + }); + res.end(body); +} + +function sendChatSse(res: ServerResponse, content: string): void { + const chunk = JSON.stringify({ + id: "chatcmpl-fake-openai-compatible", + object: "chat.completion.chunk", + choices: [{ index: 0, delta: { role: "assistant", content }, finish_reason: null }], + }); + const doneChunk = JSON.stringify({ + id: "chatcmpl-fake-openai-compatible", + object: "chat.completion.chunk", + choices: [{ index: 0, delta: {}, finish_reason: "stop" }], + }); + const body = `data: ${chunk}\n\ndata: ${doneChunk}\n\ndata: [DONE]\n\n`; + res.writeHead(200, { + "Content-Type": "text/event-stream", + "Content-Length": Buffer.byteLength(body), + }); + res.end(body); +} + +function sendResponseSse(res: ServerResponse, text: string): void { + const body = [ + "event: response.output_text.delta", + `data: ${JSON.stringify({ delta: text })}`, + "", + "event: response.completed", + "data: {}", + "", + ].join("\n"); + res.writeHead(200, { + "Content-Type": "text/event-stream", + "Content-Length": Buffer.byteLength(body), + }); + res.end(body); +} + +function isAuthOk(req: IncomingMessage): boolean { + if (!requireAuth) return true; + return req.headers.authorization === `Bearer ${apiKey}`; +} + +function requestPath(req: IncomingMessage): string { + return new URL(req.url || "/", "http://fake-openai-compatible.local").pathname; +} + +function readBody(req: IncomingMessage): Promise { + return new Promise((resolve) => { + const chunks: Buffer[] = []; + req.on("data", (chunk: Buffer) => chunks.push(chunk)); + req.on("end", () => resolve(Buffer.concat(chunks))); + }); +} + +function parseJsonBody(raw: Buffer): JsonObject { + if (raw.length === 0) return {}; + try { + const parsed = JSON.parse(raw.toString("utf8")); + return parsed && typeof parsed === "object" && !Array.isArray(parsed) ? parsed : {}; + } catch { + return {}; + } +} + +const server = createServer(async (req, res) => { + const path = requestPath(req); + + if (req.method === "GET" && ["/v1/models", "/models"].includes(path)) { + log(`GET ${path}`); + recordRequest({ method: "GET", path, bodyBytes: 0 }); + sendJson(res, 200, { object: "list", data: [{ id: model, object: "model" }] }); + return; + } + + const raw = await readBody(req); + const payload = parseJsonBody(raw); + const auth = isAuthOk(req) ? "ok" : "missing"; + recordRequest({ + method: req.method || "GET", + path, + bodyBytes: raw.length, + auth, + model: payload.model, + stream: Boolean(payload.stream), + }); + + if (req.method === "POST" && ["/v1/chat/completions", "/chat/completions"].includes(path)) { + log( + `POST ${path} auth=${auth} model=${String(payload.model || "")} stream=${Boolean(payload.stream)}`, + ); + if (!isAuthOk(req)) { + sendJson(res, 401, { error: { message: "missing bearer credential" } }); + return; + } + if (payload.stream) { + sendChatSse(res, chatContent); + return; + } + sendJson(res, 200, { + id: "chatcmpl-fake-openai-compatible", + object: "chat.completion", + choices: [ + { + index: 0, + message: { role: "assistant", content: chatContent }, + finish_reason: "stop", + }, + ], + }); + return; + } + + if (req.method === "POST" && ["/v1/responses", "/responses"].includes(path)) { + log(`POST ${path} auth=${auth} stream=${Boolean(payload.stream)}`); + if (!isAuthOk(req)) { + sendJson(res, 401, { error: { message: "missing bearer credential" } }); + return; + } + if (payload.stream) { + sendResponseSse(res, responseText); + return; + } + sendJson(res, 200, { + id: "resp-fake-openai-compatible", + object: "response", + output: [ + { + type: "message", + role: "assistant", + content: [{ type: "output_text", text: responseText }], + }, + ], + }); + return; + } + + sendJson(res, 404, { error: { message: "not found" } }); +}); + +server.listen(port, host, () => { + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("fake OpenAI-compatible server did not bind to a TCP port"); + } + if (portFile) writeFileSync(portFile, String(address.port)); + log(`READY host=${host} port=${address.port} model=${model}`); +}); + +for (const signal of ["SIGINT", "SIGTERM"] as const) { + process.on(signal, () => { + server.close(() => process.exit(0)); + setTimeout(() => process.exit(0), 500).unref(); + }); +} diff --git a/test/e2e/lib/openai-compatible-api-proof.sh b/test/e2e/lib/openai-compatible-api-proof.sh new file mode 100755 index 0000000000..d1c813fbce --- /dev/null +++ b/test/e2e/lib/openai-compatible-api-proof.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +start_fake_openai_compatible_api() { + local script_dir server_script port_file ready_host public_host + script_dir="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" + server_script="${script_dir}/fake-openai-compatible-api.mts" + port_file="${FAKE_OPENAI_PORT_FILE:-$(mktemp)}" + ready_host="${FAKE_OPENAI_READY_HOST:-127.0.0.1}" + + : "${FAKE_OPENAI_HOST:=127.0.0.1}" + : "${FAKE_OPENAI_PORT:=0}" + : "${FAKE_OPENAI_MODEL:=test-model}" + : "${FAKE_OPENAI_LOG:=$(mktemp)}" + + rm -f "$port_file" + : >"$FAKE_OPENAI_LOG" + + NEMOCLAW_FAKE_OPENAI_HOST="$FAKE_OPENAI_HOST" \ + NEMOCLAW_FAKE_OPENAI_PORT="$FAKE_OPENAI_PORT" \ + NEMOCLAW_FAKE_OPENAI_PORT_FILE="$port_file" \ + NEMOCLAW_FAKE_OPENAI_LOG_FILE="$FAKE_OPENAI_LOG" \ + NEMOCLAW_FAKE_OPENAI_MODEL="$FAKE_OPENAI_MODEL" \ + NEMOCLAW_FAKE_OPENAI_API_KEY="${FAKE_OPENAI_API_KEY:-}" \ + NEMOCLAW_FAKE_OPENAI_REQUIRE_AUTH="${FAKE_OPENAI_REQUIRE_AUTH:-0}" \ + NEMOCLAW_FAKE_OPENAI_CHAT_CONTENT="${FAKE_OPENAI_CHAT_CONTENT:-ok}" \ + NEMOCLAW_FAKE_OPENAI_RESPONSE_TEXT="${FAKE_OPENAI_RESPONSE_TEXT:-${FAKE_OPENAI_CHAT_CONTENT:-ok}}" \ + node --experimental-strip-types "$server_script" & + FAKE_OPENAI_PID="$!" + + for _ in $(seq 1 "${FAKE_OPENAI_READY_ATTEMPTS:-30}"); do + if [ -s "$port_file" ]; then + FAKE_OPENAI_PORT="$(cat "$port_file")" + if curl -sf "http://${ready_host}:${FAKE_OPENAI_PORT}/v1/models" >/dev/null 2>&1; then + public_host="${FAKE_OPENAI_PUBLIC_HOST:-$FAKE_OPENAI_HOST}" + if [ "$public_host" = "0.0.0.0" ]; then + public_host="127.0.0.1" + fi + FAKE_OPENAI_BASE_URL="http://${public_host}:${FAKE_OPENAI_PORT}/v1" + rm -f "$port_file" + export FAKE_OPENAI_BASE_URL FAKE_OPENAI_PID FAKE_OPENAI_PORT + return 0 + fi + fi + sleep 1 + done + + stop_fake_openai_compatible_api + rm -f "$port_file" + return 1 +} + +stop_fake_openai_compatible_api() { + if [ -n "${FAKE_OPENAI_PID:-}" ] && kill -0 "$FAKE_OPENAI_PID" 2>/dev/null; then + kill "$FAKE_OPENAI_PID" 2>/dev/null || true + wait "$FAKE_OPENAI_PID" 2>/dev/null || true + fi + FAKE_OPENAI_PID="" +} diff --git a/test/e2e/test-concurrent-gateway-ports.sh b/test/e2e/test-concurrent-gateway-ports.sh index af3db85d7c..268ebb3a84 100755 --- a/test/e2e/test-concurrent-gateway-ports.sh +++ b/test/e2e/test-concurrent-gateway-ports.sh @@ -62,11 +62,12 @@ DASHBOARD_PORT_A=18789 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" REPO_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" -FAKE_HOST="127.0.0.1" -FAKE_PORT="${NEMOCLAW_E2E_FAKE_PORT:-18180}" -FAKE_BASE_URL="http://${FAKE_HOST}:${FAKE_PORT}/v1" -FAKE_LOG="$(mktemp)" -FAKE_PID="" +# shellcheck source=test/e2e/lib/openai-compatible-api-proof.sh +source "${SCRIPT_DIR}/lib/openai-compatible-api-proof.sh" +FAKE_OPENAI_HOST="127.0.0.1" +FAKE_OPENAI_PORT="${NEMOCLAW_E2E_FAKE_PORT:-18180}" +FAKE_OPENAI_LOG="$(mktemp)" +FAKE_BASE_URL="http://${FAKE_OPENAI_HOST}:${FAKE_OPENAI_PORT}/v1" if command -v node >/dev/null 2>&1 && [ -f "$REPO_ROOT/bin/nemoclaw.js" ]; then NEMOCLAW_CMD=(node "$REPO_ROOT/bin/nemoclaw.js") @@ -76,83 +77,19 @@ fi # shellcheck disable=SC2329 cleanup() { - if [ -n "$FAKE_PID" ] && kill -0 "$FAKE_PID" 2>/dev/null; then - kill "$FAKE_PID" 2>/dev/null || true - wait "$FAKE_PID" 2>/dev/null || true - fi - rm -f "$FAKE_LOG" + stop_fake_openai_compatible_api + rm -f "$FAKE_OPENAI_LOG" } trap cleanup EXIT start_fake_openai() { - python3 - "$FAKE_HOST" "$FAKE_PORT" >"$FAKE_LOG" 2>&1 <<'PY' & -import json -import sys -from http.server import BaseHTTPRequestHandler, HTTPServer - -HOST = sys.argv[1] -PORT = int(sys.argv[2]) - - -class Handler(BaseHTTPRequestHandler): - def _send(self, status, payload): - body = json.dumps(payload).encode("utf-8") - self.send_response(status) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(body))) - self.end_headers() - self.wfile.write(body) - - def log_message(self, format, *args): - return - - def do_GET(self): - if self.path in ("/v1/models", "/models"): - self._send(200, {"data": [{"id": "test-model", "object": "model"}]}) - return - self._send(404, {"error": {"message": "not found"}}) - - def do_POST(self): - length = int(self.headers.get("Content-Length", "0")) - if length: - self.rfile.read(length) - if self.path in ("/v1/chat/completions", "/chat/completions"): - self._send( - 200, - { - "id": "chatcmpl-test", - "object": "chat.completion", - "choices": [{"index": 0, "message": {"role": "assistant", "content": "ok"}, "finish_reason": "stop"}], - }, - ) - return - if self.path in ("/v1/responses", "/responses"): - self._send( - 200, - { - "id": "resp-test", - "object": "response", - "output": [{"type": "message", "role": "assistant", "content": [{"type": "output_text", "text": "ok"}]}], - }, - ) - return - self._send(404, {"error": {"message": "not found"}}) - - -HTTPServer((HOST, PORT), Handler).serve_forever() -PY - FAKE_PID=$! - - for _ in $(seq 1 20); do - if curl -sf "${FAKE_BASE_URL}/models" >/dev/null 2>&1; then - info "Fake OpenAI server up on ${FAKE_BASE_URL} (pid ${FAKE_PID})" - return 0 - fi - sleep 1 - done - - fail "Fake OpenAI server did not become ready on ${FAKE_BASE_URL}; see ${FAKE_LOG}" - cat "$FAKE_LOG" + if start_fake_openai_compatible_api; then + FAKE_BASE_URL="$FAKE_OPENAI_BASE_URL" + info "Fake OpenAI server up on ${FAKE_BASE_URL} (pid ${FAKE_OPENAI_PID})" + return 0 + fi + fail "Fake OpenAI server did not become ready on ${FAKE_BASE_URL}; see ${FAKE_OPENAI_LOG}" + cat "$FAKE_OPENAI_LOG" exit 1 } diff --git a/test/e2e/test-double-onboard.sh b/test/e2e/test-double-onboard.sh index 5def539b4d..d0a5269bb1 100755 --- a/test/e2e/test-double-onboard.sh +++ b/test/e2e/test-double-onboard.sh @@ -179,11 +179,12 @@ ALT_GATEWAY_NAME="e2e-double-alt" REGISTRY="$HOME/.nemoclaw/sandboxes.json" SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" REPO_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" -FAKE_HOST="127.0.0.1" -FAKE_PORT="${NEMOCLAW_FAKE_PORT:-18080}" -FAKE_BASE_URL="http://${FAKE_HOST}:${FAKE_PORT}/v1" -FAKE_LOG="$(mktemp)" -FAKE_PID="" +# shellcheck source=test/e2e/lib/openai-compatible-api-proof.sh +source "${SCRIPT_DIR}/lib/openai-compatible-api-proof.sh" +FAKE_OPENAI_HOST="127.0.0.1" +FAKE_OPENAI_PORT="${NEMOCLAW_FAKE_PORT:-18080}" +FAKE_OPENAI_LOG="$(mktemp)" +FAKE_BASE_URL="http://${FAKE_OPENAI_HOST}:${FAKE_OPENAI_PORT}/v1" if command -v node >/dev/null 2>&1 && [ -f "$REPO_ROOT/bin/nemoclaw.js" ]; then NEMOCLAW_CMD=(node "$REPO_ROOT/bin/nemoclaw.js") @@ -193,81 +194,14 @@ fi # shellcheck disable=SC2329 cleanup() { - if [ -n "$FAKE_PID" ] && kill -0 "$FAKE_PID" 2>/dev/null; then - kill "$FAKE_PID" 2>/dev/null || true - wait "$FAKE_PID" 2>/dev/null || true - fi - rm -f "$FAKE_LOG" + stop_fake_openai_compatible_api + rm -f "$FAKE_OPENAI_LOG" } trap cleanup EXIT start_fake_openai() { - python3 - "$FAKE_HOST" "$FAKE_PORT" >"$FAKE_LOG" 2>&1 <<'PY' & -import json -import sys -from http.server import BaseHTTPRequestHandler, HTTPServer - -HOST = sys.argv[1] -PORT = int(sys.argv[2]) - - -class Handler(BaseHTTPRequestHandler): - def _send(self, status, payload): - body = json.dumps(payload).encode("utf-8") - self.send_response(status) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(body))) - self.end_headers() - self.wfile.write(body) - - def log_message(self, format, *args): - return - - def do_GET(self): - if self.path in ("/v1/models", "/models"): - self._send(200, {"data": [{"id": "test-model", "object": "model"}]}) - return - self._send(404, {"error": {"message": "not found"}}) - - def do_POST(self): - length = int(self.headers.get("Content-Length", "0")) - if length: - self.rfile.read(length) - if self.path in ("/v1/chat/completions", "/chat/completions"): - self._send( - 200, - { - "id": "chatcmpl-test", - "object": "chat.completion", - "choices": [{"index": 0, "message": {"role": "assistant", "content": "ok"}, "finish_reason": "stop"}], - }, - ) - return - if self.path in ("/v1/responses", "/responses"): - self._send( - 200, - { - "id": "resp-test", - "object": "response", - "output": [{"type": "message", "role": "assistant", "content": [{"type": "output_text", "text": "ok"}]}], - }, - ) - return - self._send(404, {"error": {"message": "not found"}}) - - -HTTPServer((HOST, PORT), Handler).serve_forever() -PY - FAKE_PID=$! - - for _ in $(seq 1 20); do - if curl -sf "${FAKE_BASE_URL}/models" >/dev/null 2>&1; then - return 0 - fi - sleep 1 - done - - return 1 + start_fake_openai_compatible_api || return 1 + FAKE_BASE_URL="$FAKE_OPENAI_BASE_URL" } # TODO(#2562): replace shell timeout with structured timeout once unified abstraction lands @@ -438,7 +372,7 @@ if start_fake_openai; then else fail "Failed to start fake OpenAI-compatible endpoint" info "Fake server log:" - sed 's/^/ /' "$FAKE_LOG" + sed 's/^/ /' "$FAKE_OPENAI_LOG" exit 1 fi diff --git a/test/e2e/test-issue-2478-crash-loop-recovery.sh b/test/e2e/test-issue-2478-crash-loop-recovery.sh index 149495ad0e..e6f91a9171 100755 --- a/test/e2e/test-issue-2478-crash-loop-recovery.sh +++ b/test/e2e/test-issue-2478-crash-loop-recovery.sh @@ -44,7 +44,7 @@ # # Prerequisites: # - Docker running -# - python3 + curl for the local OpenAI-compatible mock endpoint +# - node + curl for the local OpenAI-compatible mock endpoint # # Environment variables: # NEMOCLAW_NON_INTERACTIVE=1 — required @@ -99,13 +99,15 @@ SANDBOX_NAME="${NEMOCLAW_SANDBOX_NAME:-e2e-2478}" CRASH_CYCLES="${NEMOCLAW_E2E_CRASH_CYCLES:-5}" SOAK_SECONDS="${NEMOCLAW_E2E_SOAK_SECONDS:-300}" DASHBOARD_PORT="${NEMOCLAW_DASHBOARD_PORT:-18789}" -REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")/../.." && pwd)" +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" +REPO_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" +# shellcheck source=test/e2e/lib/openai-compatible-api-proof.sh +source "${SCRIPT_DIR}/lib/openai-compatible-api-proof.sh" USE_COMPAT_MOCK="${NEMOCLAW_E2E_USE_COMPAT_MOCK:-1}" COMPAT_MOCK_PORT="${NEMOCLAW_COMPAT_MOCK_PORT:-18191}" COMPAT_MODEL="${NEMOCLAW_COMPAT_MODEL:-test-model}" COMPATIBLE_KEY="${COMPATIBLE_API_KEY:-nemoclaw-e2e-compatible-key}" COMPAT_MOCK_LOG="${NEMOCLAW_COMPAT_MOCK_LOG:-/tmp/nemoclaw-2478-compatible-endpoint.log}" -COMPAT_MOCK_PID="" # ── Helpers ────────────────────────────────────────────────────── @@ -133,134 +135,20 @@ host_ip_for_sandbox() { } stop_compat_mock() { - if [ -n "${COMPAT_MOCK_PID:-}" ] && kill -0 "$COMPAT_MOCK_PID" 2>/dev/null; then - kill "$COMPAT_MOCK_PID" 2>/dev/null || true - wait "$COMPAT_MOCK_PID" 2>/dev/null || true - fi - COMPAT_MOCK_PID="" + stop_fake_openai_compatible_api } trap stop_compat_mock EXIT start_compat_mock() { - : >"$COMPAT_MOCK_LOG" - python3 - "$COMPAT_MOCK_PORT" "$COMPAT_MODEL" "$COMPATIBLE_KEY" >"$COMPAT_MOCK_LOG" 2>&1 <<'PY' & -import json -import sys -from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer - -port = int(sys.argv[1]) -model = sys.argv[2] -api_key = sys.argv[3] - - -class Handler(BaseHTTPRequestHandler): - def log_message(self, fmt, *args): - return - - def _path(self): - return self.path.split("?", 1)[0] - - def _send(self, status, payload): - body = json.dumps(payload).encode("utf-8") - self.send_response(status) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(body))) - self.end_headers() - self.wfile.write(body) - - def _send_chat_sse(self, content): - chunk = json.dumps({ - "id": "chatcmpl-2478-mock", - "object": "chat.completion.chunk", - "choices": [{"index": 0, "delta": {"role": "assistant", "content": content}, "finish_reason": None}], - }) - done_chunk = json.dumps({ - "id": "chatcmpl-2478-mock", - "object": "chat.completion.chunk", - "choices": [{"index": 0, "delta": {}, "finish_reason": "stop"}], - }) - body = ("data: %s\n\ndata: %s\n\ndata: [DONE]\n\n" % (chunk, done_chunk)).encode("utf-8") - self.send_response(200) - self.send_header("Content-Type", "text/event-stream") - self.send_header("Content-Length", str(len(body))) - self.end_headers() - self.wfile.write(body) - - def _auth_ok(self): - return self.headers.get("Authorization", "") == "Bearer " + api_key - - def do_GET(self): - if self._path() in ("/v1/models", "/models"): - print("GET %s" % self._path(), flush=True) - self._send(200, {"object": "list", "data": [{"id": model, "object": "model"}]}) - return - self._send(404, {"error": {"message": "not found"}}) - - def do_POST(self): - length = int(self.headers.get("Content-Length", "0") or "0") - raw = self.rfile.read(length) if length else b"" - try: - payload = json.loads(raw.decode("utf-8") or "{}") - except Exception: - payload = {} - - if self._path() in ("/v1/chat/completions", "/chat/completions"): - print( - "POST %s auth=%s model=%s stream=%s" - % (self._path(), "ok" if self._auth_ok() else "missing", payload.get("model"), payload.get("stream")), - flush=True, - ) - if not self._auth_ok(): - self._send(401, {"error": {"message": "missing bearer credential"}}) - return - if payload.get("stream"): - self._send_chat_sse("PONG from issue-2478 mock") - return - self._send(200, { - "id": "chatcmpl-2478-mock", - "object": "chat.completion", - "choices": [{ - "index": 0, - "message": {"role": "assistant", "content": "PONG from issue-2478 mock"}, - "finish_reason": "stop", - }], - }) - return - - if self._path() in ("/v1/responses", "/responses"): - print( - "POST %s auth=%s stream=%s" - % (self._path(), "ok" if self._auth_ok() else "missing", payload.get("stream")), - flush=True, - ) - if not self._auth_ok(): - self._send(401, {"error": {"message": "missing bearer credential"}}) - return - self._send(200, { - "id": "resp-2478-mock", - "object": "response", - "output": [{ - "type": "message", - "role": "assistant", - "content": [{"type": "output_text", "text": "PONG from issue-2478 mock"}], - }], - }) - return - - self._send(404, {"error": {"message": "not found"}}) - - -ThreadingHTTPServer(("0.0.0.0", port), Handler).serve_forever() -PY - COMPAT_MOCK_PID=$! - - for _ in $(seq 1 30); do - if curl -sf "http://127.0.0.1:${COMPAT_MOCK_PORT}/v1/models" >/dev/null 2>&1; then - return 0 - fi - sleep 1 - done - return 1 + export FAKE_OPENAI_HOST="0.0.0.0" + export FAKE_OPENAI_PORT="$COMPAT_MOCK_PORT" + export FAKE_OPENAI_MODEL="$COMPAT_MODEL" + export FAKE_OPENAI_API_KEY="$COMPATIBLE_KEY" + export FAKE_OPENAI_REQUIRE_AUTH="1" + export FAKE_OPENAI_CHAT_CONTENT="PONG from issue-2478 mock" + export FAKE_OPENAI_RESPONSE_TEXT="PONG from issue-2478 mock" + export FAKE_OPENAI_LOG="$COMPAT_MOCK_LOG" + start_fake_openai_compatible_api || return 1 } # Run a command inside the sandbox via openshell sandbox exec. Returns @@ -524,8 +412,12 @@ fi pass "Docker running" if [ "$USE_COMPAT_MOCK" = "1" ]; then - if ! command -v python3 >/dev/null 2>&1; then - fail "python3 is required for the compatible endpoint mock" + if ! command -v node >/dev/null 2>&1; then + fail "node is required for the compatible endpoint mock" + exit 1 + fi + if ! node --experimental-strip-types -e "" >/dev/null 2>&1; then + fail "node with --experimental-strip-types support is required for the compatible endpoint mock" exit 1 fi if ! command -v curl >/dev/null 2>&1; then diff --git a/test/e2e/test-openshell-gateway-upgrade.sh b/test/e2e/test-openshell-gateway-upgrade.sh index 0fe6ac7911..916d2f9eaf 100755 --- a/test/e2e/test-openshell-gateway-upgrade.sh +++ b/test/e2e/test-openshell-gateway-upgrade.sh @@ -50,6 +50,8 @@ fail() { SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" REPO_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" +# shellcheck source=test/e2e/lib/openai-compatible-api-proof.sh +source "${SCRIPT_DIR}/lib/openai-compatible-api-proof.sh" STATE_DIR="${NEMOCLAW_OPENSHELL_GATEWAY_STATE_DIR:-$HOME/.local/state/nemoclaw/openshell-docker-gateway}" PID_FILE="${STATE_DIR}/openshell-gateway.pid" OLD_NEMOCLAW_REF="${NEMOCLAW_OLD_NEMOCLAW_REF:-v0.0.36}" @@ -62,7 +64,6 @@ SURVIVOR_MARKER="gateway-upgrade-survivor-$(date +%s)" SURVIVOR_MARKER_PATH="/sandbox/.openclaw/workspace/nemoclaw-gateway-upgrade-marker" REGISTRY_FILE="$HOME/.nemoclaw/sandboxes.json" FAKE_BASE_URL="" -FAKE_MOCK_PID="" SURVIVOR_AGENT_PID="" load_shell_path() { @@ -257,7 +258,7 @@ PY cleanup() { set +e - cleanup_pid "$FAKE_MOCK_PID" + stop_fake_openai_compatible_api if command -v openshell >/dev/null 2>&1; then openshell sandbox delete "$SURVIVOR_SANDBOX" >/dev/null 2>&1 || true openshell gateway remove nemoclaw >/dev/null 2>&1 || true @@ -474,90 +475,14 @@ wait_for_survivor_ready() { } start_compatible_endpoint_mock() { - local tmp port_file - tmp="$(mktemp -d)" - port_file="${tmp}/port" - rm -f "$MOCK_LOG" - - python3 - "$port_file" "$MOCK_LOG" <<'PY' & -import json -import sys -from http.server import BaseHTTPRequestHandler, HTTPServer - -port_file = sys.argv[1] -log_file = sys.argv[2] - -class Handler(BaseHTTPRequestHandler): - def _send(self, status, payload): - body = json.dumps(payload).encode("utf-8") - self.send_response(status) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(body))) - self.end_headers() - self.wfile.write(body) - - def _log(self, message): - with open(log_file, "a", encoding="utf-8") as fh: - fh.write(message + "\n") - fh.flush() - - def log_message(self, _fmt, *_args): - return - - def do_GET(self): - self._log(f"GET {self.path}") - if self.path in ("/v1/models", "/models"): - self._send(200, {"data": [{"id": "test-model", "object": "model"}]}) - return - self._send(404, {"error": {"message": "not found"}}) - - def do_POST(self): - length = int(self.headers.get("Content-Length", "0")) - body = self.rfile.read(length) if length else b"" - self._log(f"POST {self.path} {body[:200].decode('utf-8', 'replace')}") - if self.path in ("/v1/chat/completions", "/chat/completions"): - self._send(200, { - "id": "chatcmpl-test", - "object": "chat.completion", - "choices": [{ - "index": 0, - "message": {"role": "assistant", "content": "ok"}, - "finish_reason": "stop", - }], - }) - return - if self.path in ("/v1/responses", "/responses"): - self._send(200, { - "id": "resp-test", - "object": "response", - "output": [{ - "type": "message", - "role": "assistant", - "content": [{"type": "output_text", "text": "ok"}], - }], - }) - return - self._send(404, {"error": {"message": "not found"}}) - -server = HTTPServer(("127.0.0.1", 0), Handler) -with open(port_file, "w", encoding="utf-8") as fh: - fh.write(str(server.server_port)) -server.serve_forever() -PY - FAKE_MOCK_PID="$!" - - for _i in $(seq 1 30); do - if [ -s "$port_file" ]; then - FAKE_BASE_URL="http://127.0.0.1:$(cat "$port_file")/v1" - if curl -sf "${FAKE_BASE_URL}/models" >/dev/null 2>&1; then - rm -rf "$tmp" - pass "Compatible endpoint mock is listening at ${FAKE_BASE_URL}" - return 0 - fi - fi - sleep 1 - done - rm -rf "$tmp" + export FAKE_OPENAI_HOST="127.0.0.1" + export FAKE_OPENAI_PORT="0" + export FAKE_OPENAI_LOG="$MOCK_LOG" + if start_fake_openai_compatible_api; then + FAKE_BASE_URL="$FAKE_OPENAI_BASE_URL" + pass "Compatible endpoint mock is listening at ${FAKE_BASE_URL}" + return 0 + fi fail "compatible endpoint mock did not start" } diff --git a/tools/advisors/git.mts b/tools/advisors/git.mts index 729783278b..c7d1706dff 100644 --- a/tools/advisors/git.mts +++ b/tools/advisors/git.mts @@ -33,13 +33,15 @@ export function getDiff(base: string, head: string, maxChars: number): string { } export function getDiffStat(base: string, head: string): string { - return gitOutput( - [ - ["diff", "--stat", `${base}...${head}`], - ["diff", "--stat", `${base}..${head}`], - ], - 1024 * 1024, - )?.trim() || ""; + return ( + gitOutput( + [ + ["diff", "--stat", `${base}...${head}`], + ["diff", "--stat", `${base}..${head}`], + ], + 1024 * 1024, + )?.trim() || "" + ); } export function getCommits(base: string, head: string): string[] { diff --git a/tools/advisors/github.mts b/tools/advisors/github.mts index 6d8450551e..699158b22a 100644 --- a/tools/advisors/github.mts +++ b/tools/advisors/github.mts @@ -20,22 +20,34 @@ export async function githubRest(apiPath: string, token: string): Promise "X-GitHub-Api-Version": "2022-11-28", }, }); - if (!response.ok) throw new Error(`GitHub REST ${apiPath} failed: ${response.status} ${await response.text()}`); - return await response.json() as T; + if (!response.ok) + throw new Error(`GitHub REST ${apiPath} failed: ${response.status} ${await response.text()}`); + return (await response.json()) as T; } -export async function githubRestPaginated(apiPath: string, token: string, limit: number): Promise { +export async function githubRestPaginated( + apiPath: string, + token: string, + limit: number, +): Promise { const results: T[] = []; for (let page = 1; results.length < limit; page += 1) { const separator = apiPath.includes("?") ? "&" : "?"; - const items = await githubRest(`${apiPath}${separator}per_page=${Math.min(100, limit - results.length)}&page=${page}`, token); + const items = await githubRest( + `${apiPath}${separator}per_page=${Math.min(100, limit - results.length)}&page=${page}`, + token, + ); results.push(...items); if (items.length < 100) break; } return results; } -export async function githubGraphql(token: string, query: string, variables: Record): Promise { +export async function githubGraphql( + token: string, + query: string, + variables: Record, +): Promise { const response = await fetch("https://api.github.com/graphql", { method: "POST", headers: { @@ -44,18 +56,30 @@ export async function githubGraphql(token: string, query: string, variables: Rec }, body: JSON.stringify({ query, variables }), }); - if (!response.ok) throw new Error(`GitHub GraphQL failed: ${response.status} ${await response.text()}`); - const payload = await response.json() as { data?: unknown; errors?: Array<{ message?: string }> }; + if (!response.ok) + throw new Error(`GitHub GraphQL failed: ${response.status} ${await response.text()}`); + const payload = (await response.json()) as { + data?: unknown; + errors?: Array<{ message?: string }>; + }; if (Array.isArray(payload.errors) && payload.errors.length > 0) { - const message = payload.errors.map((error) => error?.message || "unknown GraphQL error").join("; "); - const error = new Error(`GitHub GraphQL returned errors: ${message}`) as Error & { payload?: unknown }; + const message = payload.errors + .map((error) => error?.message || "unknown GraphQL error") + .join("; "); + const error = new Error(`GitHub GraphQL returned errors: ${message}`) as Error & { + payload?: unknown; + }; error.payload = payload; throw error; } return payload; } -export async function githubApi(apiPath: string, token: string, options: GitHubRequestOptions = {}): Promise { +export async function githubApi( + apiPath: string, + token: string, + options: GitHubRequestOptions = {}, +): Promise { // lgtm[js/file-access-to-http] Advisor workflows intentionally send normalized // artifact summaries and strictly validated dispatch inputs to GitHub APIs. // Callers construct apiPath from fixed workflow/comment endpoints, not PR text. @@ -97,10 +121,18 @@ export async function upsertStickyComment({ try { const existing = await findExistingComment(repo, pr, token, marker, userAgent); if (existing) { - await githubApi(`repos/${repo}/issues/comments/${existing.id}`, token, { method: "PATCH", body: { body }, userAgent }); + await githubApi(`repos/${repo}/issues/comments/${existing.id}`, token, { + method: "PATCH", + body: { body }, + userAgent, + }); console.log(`Updated ${label} comment on ${repo}#${pr}`); } else { - await githubApi(`repos/${repo}/issues/${pr}/comments`, token, { method: "POST", body: { body }, userAgent }); + await githubApi(`repos/${repo}/issues/${pr}/comments`, token, { + method: "POST", + body: { body }, + userAgent, + }); console.log(`Created ${label} comment on ${repo}#${pr}`); } } catch (error: unknown) { @@ -126,8 +158,14 @@ async function findExistingComment( userAgent?: string, ): Promise { for (let page = 1; ; page += 1) { - const comments = await githubApi(`repos/${repo}/issues/${pr}/comments?per_page=100&page=${page}`, token, { userAgent }); - const match = comments.find((comment) => typeof comment.body === "string" && comment.body.includes(marker)); + const comments = await githubApi( + `repos/${repo}/issues/${pr}/comments?per_page=100&page=${page}`, + token, + { userAgent }, + ); + const match = comments.find( + (comment) => typeof comment.body === "string" && comment.body.includes(marker), + ); if (match) return match; if (comments.length < 100) return undefined; } diff --git a/tools/advisors/io.mts b/tools/advisors/io.mts index 131d8e5969..b0209fa72b 100644 --- a/tools/advisors/io.mts +++ b/tools/advisors/io.mts @@ -67,5 +67,5 @@ export function readIfExists(filePath: string | undefined): string | undefined { export function readJsonIfExists(filePath: string | undefined): T | undefined { const text = readIfExists(filePath); - return text ? JSON.parse(text) as T : undefined; + return text ? (JSON.parse(text) as T) : undefined; } diff --git a/tools/advisors/json.mts b/tools/advisors/json.mts index cd1319d836..a196467d4a 100644 --- a/tools/advisors/json.mts +++ b/tools/advisors/json.mts @@ -1,11 +1,19 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -export function extractJson(text: string, rawPath: string, tag: string, label = "advisor output"): unknown { +export function extractJson( + text: string, + rawPath: string, + tag: string, + label = "advisor output", +): unknown { const trimmed = text.trim(); - const candidates = [trimmed, fenced(trimmed), tagged(trimmed, tag), balancedObject(trimmed)].filter( - (candidate): candidate is string => Boolean(candidate), - ); + const candidates = [ + trimmed, + fenced(trimmed), + tagged(trimmed, tag), + balancedObject(trimmed), + ].filter((candidate): candidate is string => Boolean(candidate)); for (const candidate of candidates) { try { @@ -17,7 +25,11 @@ export function extractJson(text: string, rawPath: string, tag: string, label = throw new Error(`Could not parse JSON from ${label}; see ${rawPath}`); } -export function enumValue(value: unknown, allowed: T, fallback: T[number]): T[number] { +export function enumValue( + value: unknown, + allowed: T, + fallback: T[number], +): T[number] { return typeof value === "string" && allowed.includes(value) ? value : fallback; } @@ -30,7 +42,11 @@ export function isRecord(value: unknown): value is Record { } export function stringArray(value: unknown): string[] { - return Array.isArray(value) ? value.filter((item): item is string => typeof item === "string" && item.trim().length > 0).map((item) => item.trim()) : []; + return Array.isArray(value) + ? value + .filter((item): item is string => typeof item === "string" && item.trim().length > 0) + .map((item) => item.trim()) + : []; } export function stringOrUndefined(value: unknown): string | undefined { diff --git a/tools/advisors/session.mts b/tools/advisors/session.mts index 8dd86acf49..6816acb75f 100644 --- a/tools/advisors/session.mts +++ b/tools/advisors/session.mts @@ -54,7 +54,9 @@ export function openAiAdvisorProviderConfig(credentialEnv: string): AdvisorProvi return { api: "openai-completions", baseUrl: "https://integrate.api.nvidia.com/v1", - models: [advisorModel(DEFAULT_ADVISOR_MODEL, "GPT-5.5", 256000, 32768, true, ["text", "image"])], + models: [ + advisorModel(DEFAULT_ADVISOR_MODEL, "GPT-5.5", 256000, 32768, true, ["text", "image"]), + ], ["api" + "Key"]: credentialEnv, } as AdvisorProviderConfig; } @@ -70,7 +72,9 @@ export function advisorModel( return { id, name, reasoning, input, cost: ZERO_COST, contextWindow, maxTokens }; } -export async function runReadOnlyAdvisor(options: RunReadOnlyAdvisorOptions): Promise { +export async function runReadOnlyAdvisor( + options: RunReadOnlyAdvisorOptions, +): Promise { fs.mkdirSync(options.configDir, { recursive: true }); const provider = options.provider || DEFAULT_ADVISOR_PROVIDER; const modelId = options.modelId || DEFAULT_ADVISOR_MODEL; @@ -136,22 +140,29 @@ export async function runReadOnlyAdvisor(options: RunReadOnlyAdvisorOptions): Pr return; } if (event.type === "tool_execution_end") { - raw.append(`[${options.logPrefix}] tool_end ${event.toolName} ${event.isError ? "error" : "ok"}\n`); + raw.append( + `[${options.logPrefix}] tool_end ${event.toolName} ${event.isError ? "error" : "ok"}\n`, + ); return; } if (event.type === "auto_retry_start") { - raw.append(`[${options.logPrefix}] retry ${event.attempt}/${event.maxAttempts}: ${event.errorMessage}\n`); + raw.append( + `[${options.logPrefix}] retry ${event.attempt}/${event.maxAttempts}: ${event.errorMessage}\n`, + ); } }); const startedAt = Date.now(); - const heartbeat = setInterval(() => { - const elapsedSeconds = Math.round((Date.now() - startedAt) / 1000); - const turnSuffix = currentTurnName ? ` current_turn=${currentTurnName}` : ""; - options.logProgress( - `Advisor SDK still running: elapsed=${elapsedSeconds}s timeout=${Math.round(options.timeoutMs / 1000)}s${turnSuffix}`, - ); - }, Math.max(options.heartbeatMs, 1000)); + const heartbeat = setInterval( + () => { + const elapsedSeconds = Math.round((Date.now() - startedAt) / 1000); + const turnSuffix = currentTurnName ? ` current_turn=${currentTurnName}` : ""; + options.logProgress( + `Advisor SDK still running: elapsed=${elapsedSeconds}s timeout=${Math.round(options.timeoutMs / 1000)}s${turnSuffix}`, + ); + }, + Math.max(options.heartbeatMs, 1000), + ); heartbeat.unref?.(); let timeout: NodeJS.Timeout | undefined; @@ -197,11 +208,15 @@ export async function runReadOnlyAdvisor(options: RunReadOnlyAdvisorOptions): Pr } const truncationNotes: string[] = []; - const droppedAssistantBytes = turnTextBuffers.reduce((total, buffer) => total + buffer.droppedBytes, 0); + const droppedAssistantBytes = turnTextBuffers.reduce( + (total, buffer) => total + buffer.droppedBytes, + 0, + ); if (droppedAssistantBytes > 0) { truncationNotes.push(``); } - if (raw.droppedBytes > 0) truncationNotes.push(``); + if (raw.droppedBytes > 0) + truncationNotes.push(``); if (truncationNotes.length > 0) raw.appendFooter(`\n${truncationNotes.join("\n")}\n`); const turnTexts = turnTextBuffers.map((buffer) => buffer.toString()); @@ -216,7 +231,13 @@ function normalizePromptTurns(promptTurns: AdvisorPromptTurn[]): AdvisorPromptTu } function sanitizeTurnName(name: string): string { - return name.trim().replace(/\s+/g, "-").replace(/[^A-Za-z0-9._-]/g, "").slice(0, 80) || "turn"; + return ( + name + .trim() + .replace(/\s+/g, "-") + .replace(/[^A-Za-z0-9._-]/g, "") + .slice(0, 80) || "turn" + ); } export class CappedBuffer { @@ -256,12 +277,18 @@ export class CappedBuffer { private trimToMaxBytes(maxBytes = this.maxBytes): void { if (Buffer.byteLength(this.value, "utf8") <= maxBytes) return; const trimmed = trimHeadToBytes(this.value, maxBytes); - this.droppedBytes += Buffer.byteLength(this.value.slice(0, this.value.length - trimmed.length), "utf8"); + this.droppedBytes += Buffer.byteLength( + this.value.slice(0, this.value.length - trimmed.length), + "utf8", + ); this.value = trimmed; } } -function prepareAdvisorConfig(provider: string, credentialEnv: string): { authStorage: AuthStorage; modelRegistry: ModelRegistry } { +function prepareAdvisorConfig( + provider: string, + credentialEnv: string, +): { authStorage: AuthStorage; modelRegistry: ModelRegistry } { const authStorage = AuthStorage.inMemory(); const modelRegistry = ModelRegistry.inMemory(authStorage); const credential = process.env[credentialEnv] || process.env.OPENAI_API_KEY; @@ -273,8 +300,14 @@ function prepareAdvisorConfig(provider: string, credentialEnv: string): { authSt } function trimHeadToBytes(value: string, maxBytes: number): string { - let removeChars = Math.min(value.length, Math.max(1, Buffer.byteLength(value, "utf8") - maxBytes)); - while (removeChars < value.length && Buffer.byteLength(value.slice(removeChars), "utf8") > maxBytes) { + let removeChars = Math.min( + value.length, + Math.max(1, Buffer.byteLength(value, "utf8") - maxBytes), + ); + while ( + removeChars < value.length && + Buffer.byteLength(value.slice(removeChars), "utf8") > maxBytes + ) { removeChars += 1; } return value.slice(removeChars); diff --git a/tools/e2e-advisor/analyze.mts b/tools/e2e-advisor/analyze.mts index c81ac21768..e3e73de67f 100755 --- a/tools/e2e-advisor/analyze.mts +++ b/tools/e2e-advisor/analyze.mts @@ -7,9 +7,27 @@ import path from "node:path"; import { pathToFileURL } from "node:url"; import { getChangedFiles, getDiff } from "../advisors/git.mts"; -import { advisorArtifactPaths, parseArgs, parsePositiveInt, readJson, writeJson, type AdvisorArtifactPaths } from "../advisors/io.mts"; -import { dropUndefinedValues, extractJson, recordItems, stringOrUndefined } from "../advisors/json.mts"; -import { DEFAULT_ADVISOR_MODEL, DEFAULT_ADVISOR_PROVIDER, READ_ONLY_TOOLS, type RunAdvisorResult, runReadOnlyAdvisor } from "../advisors/session.mts"; +import { + type AdvisorArtifactPaths, + advisorArtifactPaths, + parseArgs, + parsePositiveInt, + readJson, + writeJson, +} from "../advisors/io.mts"; +import { + dropUndefinedValues, + extractJson, + recordItems, + stringOrUndefined, +} from "../advisors/json.mts"; +import { + DEFAULT_ADVISOR_MODEL, + DEFAULT_ADVISOR_PROVIDER, + READ_ONLY_TOOLS, + type RunAdvisorResult, + runReadOnlyAdvisor, +} from "../advisors/session.mts"; const root = process.cwd(); const ADVISOR_PROVIDER = DEFAULT_ADVISOR_PROVIDER; @@ -80,10 +98,14 @@ async function main(): Promise { const artifacts = artifactPaths(outDir); // Keep generated advisor credential config outside uploaded artifacts. const configDir = - process.env.E2E_ADVISOR_CONFIG_DIR || path.join("/tmp", `nemoclaw-e2e-advisor-config-${process.pid}`); + process.env.E2E_ADVISOR_CONFIG_DIR || + path.join("/tmp", `nemoclaw-e2e-advisor-config-${process.pid}`); const timeoutMs = parsePositiveInt(process.env.E2E_ADVISOR_TIMEOUT_MS, 900000); const heartbeatMs = parsePositiveInt(process.env.E2E_ADVISOR_HEARTBEAT_MS, 60000); - const maxCaptureBytes = parsePositiveInt(process.env.E2E_ADVISOR_MAX_CAPTURE_BYTES, 5 * 1024 * 1024); + const maxCaptureBytes = parsePositiveInt( + process.env.E2E_ADVISOR_MAX_CAPTURE_BYTES, + 5 * 1024 * 1024, + ); fs.mkdirSync(outDir, { recursive: true }); @@ -99,8 +121,10 @@ async function main(): Promise { logProgress(`Wrote advisor prompt: ${prompt.length} character(s) at ${artifacts.prompt}`); const metadata = { baseRef, headRef, changedFiles }; - const writeFailure = (reason: string): void => writeUnavailableArtifacts(artifacts, metadata, reason, true); - const writeUnavailable = (reason: string): void => writeUnavailableArtifacts(artifacts, metadata, reason, false); + const writeFailure = (reason: string): void => + writeUnavailableArtifacts(artifacts, metadata, reason, true); + const writeUnavailable = (reason: string): void => + writeUnavailableArtifacts(artifacts, metadata, reason, false); if (process.env.E2E_ADVISOR_RUN_ANALYSIS === "0") { writeUnavailable("E2E_ADVISOR_RUN_ANALYSIS=0"); @@ -108,7 +132,9 @@ async function main(): Promise { } logProgress(`Launching advisor SDK: provider=${ADVISOR_PROVIDER} model=${ADVISOR_MODEL}`); - logProgress(`Advisor tools enabled: ${READ_ONLY_TOOLS.join(",")}; repository commands remain disabled by prompt policy`); + logProgress( + `Advisor tools enabled: ${READ_ONLY_TOOLS.join(",")}; repository commands remain disabled by prompt policy`, + ); let sdkResult: RunAdvisorResult | undefined; try { @@ -141,7 +167,10 @@ async function main(): Promise { let result: AdvisorResult; try { - result = normalizeAdvisorResult(extractJson(sdkResult.text || sdkResult.raw, artifacts.raw, "e2e_advisor_json"), metadata); + result = normalizeAdvisorResult( + extractJson(sdkResult.text || sdkResult.raw, artifacts.raw, "e2e_advisor_json"), + metadata, + ); } catch (error: unknown) { writeFailure(error instanceof Error ? error.message : String(error)); process.exit(1); @@ -158,11 +187,24 @@ function artifactPaths(outDir: string): ArtifactPaths { return advisorArtifactPaths(outDir, "e2e-advisor"); } -function writeUnavailableArtifacts(paths: ArtifactPaths, metadata: AdvisorMetadata, reason: string, failed: boolean): void { +function writeUnavailableArtifacts( + paths: ArtifactPaths, + metadata: AdvisorMetadata, + reason: string, + failed: boolean, +): void { const result = unavailableResult(metadata, reason, failed); - writeJson(paths.result, failed ? { failed: true, reason, promptPath: paths.prompt, rawPath: paths.raw } : { skipped: true, reason, promptPath: paths.prompt }); + writeJson( + paths.result, + failed + ? { failed: true, reason, promptPath: paths.prompt, rawPath: paths.raw } + : { skipped: true, reason, promptPath: paths.prompt }, + ); writeJson(paths.finalResult, result); - fs.writeFileSync(paths.summary, `# E2E Recommendation Advisor\n\n${failed ? "Failed" : "Skipped"}: ${reason}\n`); + fs.writeFileSync( + paths.summary, + `# E2E Recommendation Advisor\n\n${failed ? "Failed" : "Skipped"}: ${reason}\n`, + ); if (failed) { console.error(`Advisor analysis failed: ${reason}`); } @@ -241,7 +283,10 @@ function normalizeAdvisorResult(result: unknown, metadata: AdvisorMetadata): Adv requiredTests: sanitizeTests(object.requiredTests), optionalTests: sanitizeTests(object.optionalTests), newE2eRecommendations: sanitizeNewRecommendations(object.newE2eRecommendations), - noE2eReason: typeof object.noE2eReason === "string" || object.noE2eReason === null ? object.noE2eReason : null, + noE2eReason: + typeof object.noE2eReason === "string" || object.noE2eReason === null + ? object.noE2eReason + : null, confidence: isConfidence(object.confidence) ? object.confidence : "medium", }; @@ -259,7 +304,9 @@ function sanitizeDomains(value: unknown): AdvisorDomain[] { domain: stringOrUndefined(item.domain), reason: stringOrUndefined(item.reason), confidence: isConfidence(item.confidence) ? item.confidence : "medium", - matchedFiles: Array.isArray(item.matchedFiles) ? item.matchedFiles.filter((file): file is string => typeof file === "string") : [], + matchedFiles: Array.isArray(item.matchedFiles) + ? item.matchedFiles.filter((file): file is string => typeof file === "string") + : [], })) .filter((item) => item.domain && item.reason); } @@ -346,7 +393,11 @@ function renderSummary(result: AdvisorResult): string { return `${lines.join("\n")}\n`; } -function unavailableResult(metadata: AdvisorMetadata, reason: string, failed: boolean): AdvisorResult { +function unavailableResult( + metadata: AdvisorMetadata, + reason: string, + failed: boolean, +): AdvisorResult { return { version: 1, baseRef: metadata.baseRef, diff --git a/tools/e2e-advisor/comment.mts b/tools/e2e-advisor/comment.mts index 5eabd489ef..c3aed15d75 100644 --- a/tools/e2e-advisor/comment.mts +++ b/tools/e2e-advisor/comment.mts @@ -32,9 +32,10 @@ const summaryPath = args.summary || "artifacts/e2e-advisor/e2e-advisor-summary.m const resultPath = args.result || "artifacts/e2e-advisor/e2e-advisor-final-result.json"; const dispatchPath = args.dispatch || "artifacts/e2e-advisor/e2e-advisor-dispatch-result.json"; const token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; -const runUrl = process.env.GITHUB_SERVER_URL && process.env.GITHUB_REPOSITORY && process.env.GITHUB_RUN_ID - ? `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}` - : undefined; +const runUrl = + process.env.GITHUB_SERVER_URL && process.env.GITHUB_REPOSITORY && process.env.GITHUB_RUN_ID + ? `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}` + : undefined; const marker = ""; if (!repo || !pr) { @@ -46,7 +47,8 @@ if (!token) { process.exit(0); } -const summary = readIfExists(summaryPath) || readIfExists("artifacts/e2e-advisor/e2e-advisor-summary.md"); +const summary = + readIfExists(summaryPath) || readIfExists("artifacts/e2e-advisor/e2e-advisor-summary.md"); if (!summary) { throw new Error(`No advisor summary found at ${summaryPath}`); } @@ -55,7 +57,15 @@ const result = readJsonIfExists(resultPath); const dispatch = readJsonIfExists(dispatchPath); const body = buildComment({ summary, result, dispatch, runUrl, marker }); -await upsertStickyComment({ repo, pr, token, marker, body, label: "E2E advisor", userAgent: "nemoclaw-e2e-advisor" }); +await upsertStickyComment({ + repo, + pr, + token, + marker, + body, + label: "E2E advisor", + userAgent: "nemoclaw-e2e-advisor", +}); function buildComment({ summary, @@ -72,12 +82,10 @@ function buildComment({ }): string { const requiredTests = Array.isArray(result?.requiredTests) ? result.requiredTests : []; const optionalTests = Array.isArray(result?.optionalTests) ? result.optionalTests : []; - const requiredLine = requiredTests.length > 0 - ? requiredTests.map((test) => `\`${test.id}\``).join(", ") - : "_None_"; - const optionalLine = optionalTests.length > 0 - ? optionalTests.map((test) => `\`${test.id}\``).join(", ") - : "_None_"; + const requiredLine = + requiredTests.length > 0 ? requiredTests.map((test) => `\`${test.id}\``).join(", ") : "_None_"; + const optionalLine = + optionalTests.length > 0 ? optionalTests.map((test) => `\`${test.id}\``).join(", ") : "_None_"; const dispatchHint = result?.dispatchHint?.jobsInput ? `\n\n**Dispatch hint:** \`${result.dispatchHint.jobsInput}\`` : ""; @@ -104,9 +112,10 @@ function renderAutoDispatch(dispatch: DispatchResult | undefined): string { return ""; } if (dispatch.status === "dispatched") { - const jobs = Array.isArray(dispatch.jobs) && dispatch.jobs.length > 0 - ? dispatch.jobs.map((job) => `\`${job}\``).join(", ") - : "_unknown_"; + const jobs = + Array.isArray(dispatch.jobs) && dispatch.jobs.length > 0 + ? dispatch.jobs.map((job) => `\`${job}\``).join(", ") + : "_unknown_"; const workflow = dispatch.workflow ? ` via \`${dispatch.workflow}\`` : ""; const target = dispatch.targetRef ? ` at \`${dispatch.targetRef}\`` : ""; const run = dispatch.runUrl ? ` — [nightly run](${dispatch.runUrl})` : ""; diff --git a/tools/e2e-advisor/dispatch.mts b/tools/e2e-advisor/dispatch.mts index b900003619..b5637d5de8 100644 --- a/tools/e2e-advisor/dispatch.mts +++ b/tools/e2e-advisor/dispatch.mts @@ -127,7 +127,9 @@ async function main(): Promise { token, }); } catch (error: unknown) { - console.warn(`Could not look up dispatched nightly run: ${error instanceof Error ? error.message : String(error)}`); + console.warn( + `Could not look up dispatched nightly run: ${error instanceof Error ? error.message : String(error)}`, + ); } output = { ...plan, @@ -142,7 +144,9 @@ async function main(): Promise { // Do not write exception details to artifacts. This catch can include // network-layer failures from the GitHub API dispatch path, and those // messages are not needed in uploaded advisor artifacts. - console.error(`E2E advisor dispatch failed: ${error instanceof Error ? error.message : String(error)}`); + console.error( + `E2E advisor dispatch failed: ${error instanceof Error ? error.message : String(error)}`, + ); output = { status: "failed", reason: "E2E advisor dispatch failed; see workflow logs for details", @@ -187,7 +191,10 @@ export function planAutoDispatch({ return { ...base, reason: `event ${eventName || ""} is not pull_request` }; } if (repository !== allowedRepository) { - return { ...base, reason: `repository ${repository || ""} is not ${allowedRepository}` }; + return { + ...base, + reason: `repository ${repository || ""} is not ${allowedRepository}`, + }; } if (!pr) { return { ...base, reason: "pull_request payload was not available" }; @@ -209,10 +216,16 @@ export function planAutoDispatch({ const authorAssociation = pr.author_association || ""; const authorLogin = pr.user?.login || ""; - const allowedAssociations = parseCsv(env.E2E_ADVISOR_AUTO_DISPATCH_AUTHOR_ASSOCIATIONS || DEFAULT_ALLOWED_AUTHOR_ASSOCIATIONS); - const allowedAuthorLogins = parseCsv(env.E2E_ADVISOR_AUTO_DISPATCH_ALLOWED_AUTHORS).map(normalizeLogin); + const allowedAssociations = parseCsv( + env.E2E_ADVISOR_AUTO_DISPATCH_AUTHOR_ASSOCIATIONS || DEFAULT_ALLOWED_AUTHOR_ASSOCIATIONS, + ); + const allowedAuthorLogins = parseCsv(env.E2E_ADVISOR_AUTO_DISPATCH_ALLOWED_AUTHORS).map( + normalizeLogin, + ); const allowedByAssociation = allowedAssociations.includes(authorAssociation); - const allowedByAuthorAllowlist = Boolean(authorLogin && allowedAuthorLogins.includes(normalizeLogin(authorLogin))); + const allowedByAuthorAllowlist = Boolean( + authorLogin && allowedAuthorLogins.includes(normalizeLogin(authorLogin)), + ); if (!allowedByAssociation && !allowedByAuthorAllowlist) { return { ...base, @@ -255,7 +268,8 @@ export function planAutoDispatch({ if (jobs.length === 0) { return { ...base, - reason: "no required advisor recommendations matched dispatchable jobs in the target workflow", + reason: + "no required advisor recommendations matched dispatchable jobs in the target workflow", prNumber: pr.number, authorAssociation, authorLogin, @@ -320,12 +334,16 @@ export function extractDispatchableJobs(workflowText: string): string[] { return jobs.sort(); } -export function collectRecommendedJobs(result: AdvisorResult, targetWorkflow = DEFAULT_TARGET_WORKFLOW): string[] { +export function collectRecommendedJobs( + result: AdvisorResult, + targetWorkflow = DEFAULT_TARGET_WORKFLOW, +): string[] { const requiredTests = Array.isArray(result.requiredTests) ? result.requiredTests : []; const jobs: string[] = []; for (const test of requiredTests) { const workflow = typeof test.workflow === "string" ? path.basename(test.workflow.trim()) : ""; - const workflowMatches = !workflow || workflow === targetWorkflow || workflow === path.basename(targetWorkflow); + const workflowMatches = + !workflow || workflow === targetWorkflow || workflow === path.basename(targetWorkflow); if (!workflowMatches) continue; if (typeof test.job === "string" && test.job.trim()) jobs.push(test.job.trim()); if (typeof test.id === "string" && test.id.trim()) jobs.push(test.id.trim()); @@ -375,11 +393,15 @@ async function dispatchWorkflow({ // same-repository/OWNER-or-MEMBER gating and strict validation of the target // workflow, ref, PR number, and comma-separated E2E job names. try { - await githubApi(`repos/${safeRepo}/actions/workflows/${encodeURIComponent(safeWorkflow)}/dispatches`, token, { - method: "POST", - body: { ref: safeRef, inputs: safeInputs }, - userAgent: "nemoclaw-e2e-advisor-dispatcher", - }); + await githubApi( + `repos/${safeRepo}/actions/workflows/${encodeURIComponent(safeWorkflow)}/dispatches`, + token, + { + method: "POST", + body: { ref: safeRef, inputs: safeInputs }, + userAgent: "nemoclaw-e2e-advisor-dispatcher", + }, + ); } catch { // Do not include the response body in thrown errors. GitHub API error text // is network-controlled and this script records failures to artifact files. @@ -425,9 +447,13 @@ async function findDispatchedWorkflowRun({ if (attempt > 0) await delay(2000); let data: WorkflowRunSearchResult; try { - data = await githubApi(runsPath, token, { userAgent: "nemoclaw-e2e-advisor-dispatcher" }); + data = await githubApi(runsPath, token, { + userAgent: "nemoclaw-e2e-advisor-dispatcher", + }); } catch (error: unknown) { - console.warn(`Could not look up dispatched nightly run: ${error instanceof Error ? error.message : String(error)}`); + console.warn( + `Could not look up dispatched nightly run: ${error instanceof Error ? error.message : String(error)}`, + ); return undefined; } const match = data.workflow_runs?.find( @@ -505,7 +531,9 @@ function renderDispatchSummary(result: DispatchPlan): string { lines.push(`Jobs: ${result.jobs.map((job) => `\`${job}\``).join(", ")}`); } if (Array.isArray(result.ignoredJobs) && result.ignoredJobs.length > 0) { - lines.push(`Ignored recommendations: ${result.ignoredJobs.map((job) => `\`${job}\``).join(", ")}`); + lines.push( + `Ignored recommendations: ${result.ignoredJobs.map((job) => `\`${job}\``).join(", ")}`, + ); } lines.push(""); return `${lines.join("\n")}\n`; diff --git a/tools/e2e-advisor/scenario-comment.mts b/tools/e2e-advisor/scenario-comment.mts index 309a112913..efd71d10e9 100644 --- a/tools/e2e-advisor/scenario-comment.mts +++ b/tools/e2e-advisor/scenario-comment.mts @@ -6,17 +6,11 @@ import { pathToFileURL } from "node:url"; import { upsertStickyComment } from "../advisors/github.mts"; import { parseArgs, readIfExists, readJsonIfExists } from "../advisors/io.mts"; -import type { - ScenarioAdvisorResult, - ScenarioRecommendation, -} from "./scenarios.mts"; +import type { ScenarioAdvisorResult, ScenarioRecommendation } from "./scenarios.mts"; export const SCENARIO_ADVISOR_MARKER = ""; -if ( - process.argv[1] && - import.meta.url === pathToFileURL(process.argv[1]).href -) { +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { main().catch((error: unknown) => { console.error(error instanceof Error ? error.message : String(error)); process.exit(1); @@ -27,28 +21,20 @@ async function main(): Promise { const args = parseArgs(process.argv.slice(2)); const repo = args.repo || process.env.GITHUB_REPOSITORY; const pr = args.pr || process.env.PR_NUMBER; - const summaryPath = - args.summary || "artifacts/e2e-advisor/e2e-scenario-advisor-summary.md"; - const resultPath = - args.result || "artifacts/e2e-advisor/e2e-scenario-advisor-result.json"; + const summaryPath = args.summary || "artifacts/e2e-advisor/e2e-scenario-advisor-summary.md"; + const resultPath = args.result || "artifacts/e2e-advisor/e2e-scenario-advisor-result.json"; const token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; const runUrl = - process.env.GITHUB_SERVER_URL && - process.env.GITHUB_REPOSITORY && - process.env.GITHUB_RUN_ID + process.env.GITHUB_SERVER_URL && process.env.GITHUB_REPOSITORY && process.env.GITHUB_RUN_ID ? `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}` : undefined; if (!repo || !pr) { - console.log( - "Skipping E2E scenario advisor comment: repo or PR number not provided", - ); + console.log("Skipping E2E scenario advisor comment: repo or PR number not provided"); return; } if (!token) { - console.log( - "Skipping E2E scenario advisor comment: GITHUB_TOKEN/GH_TOKEN not provided", - ); + console.log("Skipping E2E scenario advisor comment: GITHUB_TOKEN/GH_TOKEN not provided"); return; } diff --git a/tools/e2e-advisor/scenarios.mts b/tools/e2e-advisor/scenarios.mts index b95a93bbc0..eabf509cf2 100755 --- a/tools/e2e-advisor/scenarios.mts +++ b/tools/e2e-advisor/scenarios.mts @@ -5,15 +5,22 @@ import fs from "node:fs"; import path from "node:path"; import { pathToFileURL } from "node:url"; - +// Intentionally resolves relative to the trusted advisor checkout, not the +// analyzed PR workdir. The workflow runs this script from trusted `main` while +// `process.cwd()` points at inert PR data, so normalization must not execute +// PR-local registry/runtime-support code. PRs that add or newly wire scenarios +// should use the fan-out recommendation until the trusted checkout knows their +// targeted IDs are live-supported. +import { getScenario } from "../../test/e2e-scenario/scenarios/registry.ts"; +import { liveScenarioSupport } from "../../test/e2e-scenario/scenarios/runtime-support.ts"; import { getChangedFiles, getDiff } from "../advisors/git.mts"; import { + type AdvisorArtifactPaths, advisorArtifactPaths, parseArgs, parsePositiveInt, readJson, writeJson, - type AdvisorArtifactPaths, } from "../advisors/io.mts"; import { dropUndefinedValues, @@ -29,14 +36,6 @@ import { type RunAdvisorResult, runReadOnlyAdvisor, } from "../advisors/session.mts"; -// Intentionally resolves relative to the trusted advisor checkout, not the -// analyzed PR workdir. The workflow runs this script from trusted `main` while -// `process.cwd()` points at inert PR data, so normalization must not execute -// PR-local registry/runtime-support code. PRs that add or newly wire scenarios -// should use the fan-out recommendation until the trusted checkout knows their -// targeted IDs are live-supported. -import { getScenario } from "../../test/e2e-scenario/scenarios/registry.ts"; -import { liveScenarioSupport } from "../../test/e2e-scenario/scenarios/runtime-support.ts"; const root = process.cwd(); const ADVISOR_PROVIDER = DEFAULT_ADVISOR_PROVIDER; @@ -120,10 +119,7 @@ export type ScenarioAdvisorResult = { confidence: Confidence; }; -if ( - process.argv[1] && - import.meta.url === pathToFileURL(process.argv[1]).href -) { +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { main().catch((error: unknown) => { console.error(error instanceof Error ? error.message : String(error)); process.exit(1); @@ -143,11 +139,16 @@ async function main(): Promise { path.join("/tmp", `nemoclaw-e2e-scenario-advisor-config-${process.pid}`); const timeoutMs = parsePositiveInt(process.env.E2E_SCENARIO_ADVISOR_TIMEOUT_MS, 900000); const heartbeatMs = parsePositiveInt(process.env.E2E_SCENARIO_ADVISOR_HEARTBEAT_MS, 60000); - const maxCaptureBytes = parsePositiveInt(process.env.E2E_SCENARIO_ADVISOR_MAX_CAPTURE_BYTES, 5 * 1024 * 1024); + const maxCaptureBytes = parsePositiveInt( + process.env.E2E_SCENARIO_ADVISOR_MAX_CAPTURE_BYTES, + 5 * 1024 * 1024, + ); fs.mkdirSync(outDir, { recursive: true }); - logProgress(`Starting scenario advisor analysis: base=${baseRef} head=${headRef} outDir=${outDir}`); + logProgress( + `Starting scenario advisor analysis: base=${baseRef} head=${headRef} outDir=${outDir}`, + ); const schema = readJson(schemaPath); const changedFiles = getChangedFiles(baseRef, headRef); logProgress(`Detected ${changedFiles.length} changed file(s)`); @@ -156,11 +157,15 @@ async function main(): Promise { const systemPrompt = buildSystemPrompt(schema); const prompt = buildPrompt({ baseRef, headRef, changedFiles, diff }); fs.writeFileSync(artifacts.prompt, prompt); - logProgress(`Wrote scenario advisor prompt: ${prompt.length} character(s) at ${artifacts.prompt}`); + logProgress( + `Wrote scenario advisor prompt: ${prompt.length} character(s) at ${artifacts.prompt}`, + ); const metadata = { baseRef, headRef, changedFiles }; - const writeFailure = (reason: string): void => writeUnavailableArtifacts(artifacts, metadata, reason, true); - const writeUnavailable = (reason: string): void => writeUnavailableArtifacts(artifacts, metadata, reason, false); + const writeFailure = (reason: string): void => + writeUnavailableArtifacts(artifacts, metadata, reason, true); + const writeUnavailable = (reason: string): void => + writeUnavailableArtifacts(artifacts, metadata, reason, false); if (process.env.E2E_SCENARIO_ADVISOR_RUN_ANALYSIS === "0") { writeUnavailable("E2E_SCENARIO_ADVISOR_RUN_ANALYSIS=0"); @@ -168,7 +173,9 @@ async function main(): Promise { } logProgress(`Launching advisor SDK: provider=${ADVISOR_PROVIDER} model=${ADVISOR_MODEL}`); - logProgress(`Advisor tools enabled: ${READ_ONLY_TOOLS.join(",")}; repository commands remain disabled by prompt policy`); + logProgress( + `Advisor tools enabled: ${READ_ONLY_TOOLS.join(",")}; repository commands remain disabled by prompt policy`, + ); let sdkResult: RunAdvisorResult | undefined; try { @@ -337,7 +344,10 @@ export function normalizeScenarioAdvisorResult( metadata.changedFiles, unwiredFreeStandingLiveTests, ); - const deterministicJobs = deterministicFreeStandingJobRecommendations(metadata.changedFiles, context); + const deterministicJobs = deterministicFreeStandingJobRecommendations( + metadata.changedFiles, + context, + ); const required = suppressFanout ? [] : mergeRecommendations( @@ -358,7 +368,10 @@ export function normalizeScenarioAdvisorResult( const reasonField = object.noScenarioE2eReason; const noScenarioE2eReason = suppressFanout ? missingFreeStandingLiveWiringReason(unwiredFreeStandingLiveTests) - : typeof reasonField === "string" && reasonField.trim() && required.length === 0 && optional.length === 0 + : typeof reasonField === "string" && + reasonField.trim() && + required.length === 0 && + optional.length === 0 ? reasonField.trim() : required.length === 0 && optional.length === 0 ? unwiredFreeStandingLiveTests.length > 0 @@ -371,13 +384,23 @@ export function normalizeScenarioAdvisorResult( baseRef: metadata.baseRef, headRef: metadata.headRef, changedFiles: metadata.changedFiles, - relevantChangedFiles: stringArrayWithinChanged(object.relevantChangedFiles, metadata.changedFiles), + relevantChangedFiles: stringArrayWithinChanged( + object.relevantChangedFiles, + metadata.changedFiles, + ), required, optional: optional.filter( - (candidate) => !required.some((item) => item.id === candidate.id && item.selectorType === candidate.selectorType), + (candidate) => + !required.some( + (item) => item.id === candidate.id && item.selectorType === candidate.selectorType, + ), ), noScenarioE2eReason, - confidence: enumValue<["low", "medium", "high"]>(object.confidence, ["low", "medium", "high"], "medium"), + confidence: enumValue<["low", "medium", "high"]>( + object.confidence, + ["low", "medium", "high"], + "medium", + ), }; } @@ -389,7 +412,9 @@ function readVitestWorkflowText(): string | undefined { } } -function buildScenarioNormalizationContext(vitestWorkflowText = readVitestWorkflowText()): ScenarioNormalizationContext { +function buildScenarioNormalizationContext( + vitestWorkflowText = readVitestWorkflowText(), +): ScenarioNormalizationContext { const freeStandingJobs = extractFreeStandingVitestJobs(vitestWorkflowText ?? ""); const liveTestToJobs = new Map(); for (const job of freeStandingJobs) { @@ -450,9 +475,7 @@ function shouldSuppressFanoutForUnwiredLiveTests( if (unwiredFreeStandingLiveTests.length === 0) return false; const relevantFiles = changedFiles.filter(isVitestScenarioRelevantFile); return relevantFiles.every( - (file) => - unwiredFreeStandingLiveTests.includes(file) || - file === SCENARIO_WORKFLOW_PATH, + (file) => unwiredFreeStandingLiveTests.includes(file) || file === SCENARIO_WORKFLOW_PATH, ); } diff --git a/tools/pr-review-advisor/analyze.mts b/tools/pr-review-advisor/analyze.mts index bb5be09fa9..8bdbaabaeb 100755 --- a/tools/pr-review-advisor/analyze.mts +++ b/tools/pr-review-advisor/analyze.mts @@ -6,14 +6,30 @@ import fs from "node:fs"; import path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; -import { getChangedFiles, getCommits, getDiff, getDiffStat, getHeadSha, gitOutput } from "../advisors/git.mts"; +import { + getChangedFiles, + getCommits, + getDiff, + getDiffStat, + getHeadSha, + gitOutput, +} from "../advisors/git.mts"; import { githubRest, githubRestPaginated } from "../advisors/github.mts"; import { parseArgs, parsePositiveInt, readJson, writeJson } from "../advisors/io.mts"; -import { enumValue, extractJson, getPath, isRecord, recordItems, stringArray, stringOrDefault, stringOrUndefined } from "../advisors/json.mts"; import { + enumValue, + extractJson, + getPath, + isRecord, + recordItems, + stringArray, + stringOrDefault, + stringOrUndefined, +} from "../advisors/json.mts"; +import { + type AdvisorPromptTurn, DEFAULT_ADVISOR_MODEL, DEFAULT_ADVISOR_PROVIDER, - type AdvisorPromptTurn, type RunAdvisorResult, runReadOnlyAdvisor, } from "../advisors/session.mts"; @@ -22,7 +38,10 @@ const root = process.cwd(); const ADVISOR_PROVIDER = DEFAULT_ADVISOR_PROVIDER; const ADVISOR_MODEL = DEFAULT_ADVISOR_MODEL; const ADVISOR_CREDENTIAL_ENV = ["PR", "REVIEW", "ADVISOR", "API", "KEY"].join("_"); -const SECURITY_REVIEW_SKILL_PATH = ".agents/skills/nemoclaw-maintainer-security-code-review/SKILL.md"; +const OPEN_PR_OVERLAP_LIMIT = 80; +const OPEN_PR_OVERLAP_CONCURRENCY = 6; +const SECURITY_REVIEW_SKILL_PATH = + ".agents/skills/nemoclaw-maintainer-security-code-review/SKILL.md"; const TRUSTED_SECURITY_REVIEW_SKILL_PATH = path.resolve( path.dirname(fileURLToPath(import.meta.url)), "..", @@ -59,10 +78,20 @@ const SUMMARY_RECOMMENDATIONS = [ "info_only", ] as const; const CONFIDENCES = ["low", "medium", "high"] as const; -const TEST_DEPTH_VERDICTS = ["unit_sufficient", "mocks_recommended", "runtime_validation_recommended", "unknown"] as const; +const TEST_DEPTH_VERDICTS = [ + "unit_sufficient", + "mocks_recommended", + "runtime_validation_recommended", + "unknown", +] as const; const ACCEPTANCE_STATUSES = ["met", "partial", "missing", "unknown"] as const; const SECURITY_VERDICTS = ["pass", "warning", "fail"] as const; -const SOURCE_OF_TRUTH_STATUSES = ["not_applicable", "satisfied", "needs_followup", "missing"] as const; +const SOURCE_OF_TRUTH_STATUSES = [ + "not_applicable", + "satisfied", + "needs_followup", + "missing", +] as const; type Confidence = (typeof CONFIDENCES)[number]; type SummaryRecommendation = (typeof SUMMARY_RECOMMENDATIONS)[number]; @@ -246,14 +275,20 @@ async function main(): Promise { const schemaPath = args.schema || "tools/pr-review-advisor/schema.json"; const artifacts = artifactPaths(outDir); const configDir = - process.env.PR_REVIEW_ADVISOR_CONFIG_DIR || path.join("/tmp", `nemoclaw-pr-review-advisor-config-${process.pid}`); + process.env.PR_REVIEW_ADVISOR_CONFIG_DIR || + path.join("/tmp", `nemoclaw-pr-review-advisor-config-${process.pid}`); const timeoutMs = parsePositiveInt(process.env.PR_REVIEW_ADVISOR_TIMEOUT_MS, 900000); const heartbeatMs = parsePositiveInt(process.env.PR_REVIEW_ADVISOR_HEARTBEAT_MS, 60000); - const maxCaptureBytes = parsePositiveInt(process.env.PR_REVIEW_ADVISOR_MAX_CAPTURE_BYTES, 5 * 1024 * 1024); + const maxCaptureBytes = parsePositiveInt( + process.env.PR_REVIEW_ADVISOR_MAX_CAPTURE_BYTES, + 5 * 1024 * 1024, + ); fs.mkdirSync(outDir, { recursive: true }); - logProgress(`Starting PR review advisor analysis: base=${baseRef} head=${headRef} outDir=${outDir}`); + logProgress( + `Starting PR review advisor analysis: base=${baseRef} head=${headRef} outDir=${outDir}`, + ); const schema = readJson>(schemaPath); const changedFiles = getChangedFiles(baseRef, headRef); const headSha = getHeadSha(headRef); @@ -264,15 +299,19 @@ async function main(): Promise { const promptTurns = buildPromptTurns({ metadata, diff, schema }); writePromptArtifacts({ promptDir: artifacts.promptDir, systemPrompt, promptTurns }); - const writeFailure = (reason: string): void => writeUnavailableArtifacts(artifacts, metadata, reason, true); - const writeUnavailable = (reason: string): void => writeUnavailableArtifacts(artifacts, metadata, reason, false); + const writeFailure = (reason: string): void => + writeUnavailableArtifacts(artifacts, metadata, reason, true); + const writeUnavailable = (reason: string): void => + writeUnavailableArtifacts(artifacts, metadata, reason, false); if (process.env.PR_REVIEW_ADVISOR_RUN_ANALYSIS === "0") { writeUnavailable("PR_REVIEW_ADVISOR_RUN_ANALYSIS=0"); process.exit(0); } - logProgress(`Launching PR review advisor SDK: provider=${ADVISOR_PROVIDER} model=${ADVISOR_MODEL}`); + logProgress( + `Launching PR review advisor SDK: provider=${ADVISOR_PROVIDER} model=${ADVISOR_MODEL}`, + ); let sdkResult: RunAdvisorResult | undefined; try { sdkResult = await runReadOnlyAdvisor({ @@ -299,7 +338,15 @@ async function main(): Promise { let result: ReviewAdvisorResult; try { - result = normalizeReviewResult(extractJson(sdkResult.text || sdkResult.raw, artifacts.raw, "pr_review_advisor_json", "PR review advisor output"), metadata); + result = normalizeReviewResult( + extractJson( + sdkResult.text || sdkResult.raw, + artifacts.raw, + "pr_review_advisor_json", + "PR review advisor output", + ), + metadata, + ); } catch (error: unknown) { writeFailure(error instanceof Error ? error.message : String(error)); process.exit(1); @@ -309,7 +356,10 @@ async function main(): Promise { writeJson(artifacts.finalResult, result); const summary = renderSummary(result); fs.writeFileSync(artifacts.summary, summary); - fs.writeFileSync(path.join(outDir, "pr-review-advisor-detailed-review.md"), renderDetailedReview(result)); + fs.writeFileSync( + path.join(outDir, "pr-review-advisor-detailed-review.md"), + renderDetailedReview(result), + ); console.log(summary); } @@ -333,7 +383,9 @@ function writeUnavailableArtifacts( const result = unavailableResult(metadata, reason, failed); writeJson( paths.result, - failed ? { failed: true, reason, promptPath: paths.promptDir, rawPath: paths.raw } : { skipped: true, reason, promptPath: paths.promptDir }, + failed + ? { failed: true, reason, promptPath: paths.promptDir, rawPath: paths.raw } + : { skipped: true, reason, promptPath: paths.promptDir }, ); writeJson(paths.finalResult, result); fs.writeFileSync(paths.summary, renderSummary(result)); @@ -374,16 +426,24 @@ async function collectDeterministicContext(options: { function detectRiskyAreas(changedFiles: string[]): string[] { const areas = new Set(); for (const file of changedFiles) { - if (/^(install|setup|brev-setup)\.sh$/.test(file) || /^scripts\/.*\.sh$/.test(file)) areas.add("installer/bootstrap shell"); - if (file === "src/lib/onboard.ts" || file === "bin/nemoclaw.js" || file.startsWith("scripts/")) areas.add("onboarding/host glue"); - if (file.startsWith("nemoclaw/src/blueprint/") || file.startsWith("nemoclaw-blueprint/")) areas.add("sandbox/policy/SSRF"); - if (file.startsWith(".github/workflows/") || file.includes("prek") || file.includes("dco")) areas.add("workflow/enforcement"); - if (/credential|inference|network|approval|provider/i.test(file)) areas.add("credentials/inference/network"); + if (/^(install|setup|brev-setup)\.sh$/.test(file) || /^scripts\/.*\.sh$/.test(file)) + areas.add("installer/bootstrap shell"); + if (file === "src/lib/onboard.ts" || file === "bin/nemoclaw.js" || file.startsWith("scripts/")) + areas.add("onboarding/host glue"); + if (file.startsWith("nemoclaw/src/blueprint/") || file.startsWith("nemoclaw-blueprint/")) + areas.add("sandbox/policy/SSRF"); + if (file.startsWith(".github/workflows/") || file.includes("prek") || file.includes("dco")) + areas.add("workflow/enforcement"); + if (/credential|inference|network|approval|provider/i.test(file)) + areas.add("credentials/inference/network"); } return [...areas].sort(); } -export function classifyTestDepth(changedFiles: string[], diff = ""): ReviewAdvisorResult["testDepth"] { +export function classifyTestDepth( + changedFiles: string[], + diff = "", +): ReviewAdvisorResult["testDepth"] { const sourceFiles = changedFiles.filter((file) => !isTestFile(file)); if (changedFiles.length === 0) { return { verdict: "unknown", rationale: "No changed files were detected.", suggestedTests: [] }; @@ -391,28 +451,32 @@ export function classifyTestDepth(changedFiles: string[], diff = ""): ReviewAdvi if (sourceFiles.length === 0 || sourceFiles.every(isDocsOrTestOnly)) { return { verdict: "unit_sufficient", - rationale: "Changes are limited to tests, documentation, or metadata that cannot affect runtime behavior directly.", + rationale: + "Changes are limited to tests, documentation, or metadata that cannot affect runtime behavior directly.", suggestedTests: ["Run the relevant existing unit/doc validation for the touched files."], }; } - const e2eSignals = sourceFiles.filter((file) => - file === "Dockerfile" || - file.endsWith("Dockerfile") || - /(^|\/)(install|setup|brev-setup|nemoclaw-start)\.sh$/.test(file) || - file.startsWith("nemoclaw-blueprint/policies/") || - file.startsWith("nemoclaw/src/blueprint/") || - file.startsWith("test/e2e/") || - file.includes("sandbox") || - file.includes("gateway") || - file.includes("rebuild") || - file.includes("snapshot") || - /\b(execFileSync|execSync|spawnSync|run\(|docker|openshell)\b/.test(diff), + const e2eSignals = sourceFiles.filter( + (file) => + file === "Dockerfile" || + file.endsWith("Dockerfile") || + /(^|\/)(install|setup|brev-setup|nemoclaw-start)\.sh$/.test(file) || + file.startsWith("nemoclaw-blueprint/policies/") || + file.startsWith("nemoclaw/src/blueprint/") || + file.startsWith("test/e2e/") || + file.includes("sandbox") || + file.includes("gateway") || + file.includes("rebuild") || + file.includes("snapshot") || + /\b(execFileSync|execSync|spawnSync|run\(|docker|openshell)\b/.test(diff), ); if (e2eSignals.length > 0) { return { verdict: "runtime_validation_recommended", rationale: `Runtime/sandbox/infrastructure paths need behavioral runtime validation: ${e2eSignals.slice(0, 8).join(", ")}.`, - suggestedTests: ["Add or identify targeted runtime/integration validation for the changed behavior; do not report external E2E job pass/fail here."], + suggestedTests: [ + "Add or identify targeted runtime/integration validation for the changed behavior; do not report external E2E job pass/fail here.", + ], }; } const mockSignals = sourceFiles.filter((file) => @@ -422,7 +486,9 @@ export function classifyTestDepth(changedFiles: string[], diff = ""): ReviewAdvi return { verdict: "mocks_recommended", rationale: `Changed code has I/O, state, credentials, provider, or config behavior that should be covered with behavioral mocks: ${mockSignals.slice(0, 8).join(", ")}.`, - suggestedTests: ["Add or confirm behavioral tests with mocked filesystem/network/process boundaries."], + suggestedTests: [ + "Add or confirm behavioral tests with mocked filesystem/network/process boundaries.", + ], }; } return { @@ -437,21 +503,39 @@ function isTestFile(file: string): boolean { } function isDocsOrTestOnly(file: string): boolean { - return isTestFile(file) || /\.(md|mdx|txt)$/.test(file) || file.startsWith("docs/") || file.startsWith("fern/"); + return ( + isTestFile(file) || + /\.(md|mdx|txt)$/.test(file) || + file.startsWith("docs/") || + file.startsWith("fern/") + ); } function detectWorkflowSignals(changedFiles: string[], diff: string): string[] { if (!changedFiles.some((file) => file.startsWith(".github/workflows/"))) return []; - const signals: string[] = ["Workflow files changed; review trusted-code boundary, permissions, and pinning."]; - if (/secrets\./.test(diff) || /GITHUB_TOKEN|GH_TOKEN/.test(diff)) signals.push("Secrets or GitHub tokens appear in workflow diff."); - if (/pull_request_target/.test(diff)) signals.push("pull_request_target appears in workflow diff."); - if (/permissions:\s*[\s\S]*write/.test(diff)) signals.push("Workflow requests write-scoped permissions."); - if (/npm install|pip install|curl .*\|.*sh|uv tool install/.test(diff)) signals.push("Workflow installs runtime dependencies; verify exact pins and disabled lifecycle hooks."); - if (/github\.event\.pull_request\.(title|body|head\.ref)/.test(diff)) signals.push("PR-controlled text may be interpolated into workflow expressions; verify shell safety."); + const signals: string[] = [ + "Workflow files changed; review trusted-code boundary, permissions, and pinning.", + ]; + if (/secrets\./.test(diff) || /GITHUB_TOKEN|GH_TOKEN/.test(diff)) + signals.push("Secrets or GitHub tokens appear in workflow diff."); + if (/pull_request_target/.test(diff)) + signals.push("pull_request_target appears in workflow diff."); + if (/permissions:\s*[\s\S]*write/.test(diff)) + signals.push("Workflow requests write-scoped permissions."); + if (/npm install|pip install|curl .*\|.*sh|uv tool install/.test(diff)) + signals.push( + "Workflow installs runtime dependencies; verify exact pins and disabled lifecycle hooks.", + ); + if (/github\.event\.pull_request\.(title|body|head\.ref)/.test(diff)) + signals.push( + "PR-controlled text may be interpolated into workflow expressions; verify shell safety.", + ); return signals; } -export function findRetiredE2eMigrationLedgerChanges(diff: string): RetiredE2eMigrationLedgerChange[] { +export function findRetiredE2eMigrationLedgerChanges( + diff: string, +): RetiredE2eMigrationLedgerChange[] { const retiredLedgers = new Set([ "test/e2e-scenario/migration/legacy-inventory.json", "test/e2e/docs/parity-inventory.generated.json", @@ -467,7 +551,10 @@ export function findRetiredE2eMigrationLedgerChanges(diff: string): RetiredE2eMi if (/^deleted file mode\b/m.test(header) || /^\+\+\+ \/dev\/null$/m.test(header)) continue; changes.set(file, { file, - change: /^new file mode\b/m.test(header) || /^--- \/dev\/null$/m.test(header) ? "added" : "modified", + change: + /^new file mode\b/m.test(header) || /^--- \/dev\/null$/m.test(header) + ? "added" + : "modified", }); } return [...changes.values()].sort((a, b) => a.file.localeCompare(b.file)); @@ -477,11 +564,13 @@ export function detectLocalizedPatchSignals(diff: string): LocalizedPatchSignal[ const patterns: Array<{ kind: string; regex: RegExp }> = [ { kind: "fallback/recovery/tolerance path", - regex: /\b(?:fallback\w*|recover|recovery|best[- ]?effort|workaround|compatibility|legacy|tolerant|repair|self[- ]?heal|degraded)\b/i, + regex: + /\b(?:fallback\w*|recover|recovery|best[- ]?effort|workaround|compatibility|legacy|tolerant|repair|self[- ]?heal|degraded)\b/i, }, { kind: "runtime interception or monkeypatch", - regex: /\b(?:NODE_OPTIONS|uncaughtException|unhandledRejection|process\.emit|require\.cache|prototype|monkey[- ]?patch|http\.request|https\.request|networkInterfaces)\b/i, + regex: + /\b(?:NODE_OPTIONS|uncaughtException|unhandledRejection|process\.emit|require\.cache|prototype|monkey[- ]?patch|http\.request|https\.request|networkInterfaces)\b/i, }, { kind: "silent/defaulted error handling", @@ -516,7 +605,8 @@ export function detectLocalizedPatchSignals(diff: string): LocalizedPatchSignal[ line: nextLine, kind: pattern.kind, evidence: content.slice(0, 220), - reviewRule: "If this is a localized patch, identify the invalid state, its source boundary, why the source cannot be fixed here, the regression test, and the removal condition.", + reviewRule: + "If this is a localized patch, identify the invalid state, its source boundary, why the source cannot be fixed here, the regression test, and the removal condition.", }); break; } @@ -543,16 +633,19 @@ export function computeMonolithDeltas(baseRef: string, changedFiles: string[]): return classifyMonolithDelta({ file, baseLines, headLines, delta: headLines - baseLines }); }) .filter((delta) => delta.headLines >= 400 || delta.baseLines >= 400 || delta.delta > 0) - .sort((a, b) => severityRank(b.severity) - severityRank(a.severity) || Math.abs(b.delta) - Math.abs(a.delta)); + .sort( + (a, b) => + severityRank(b.severity) - severityRank(a.severity) || + Math.abs(b.delta) - Math.abs(a.delta), + ); } -export function classifyMonolithDelta(delta: Omit): MonolithDelta { +export function classifyMonolithDelta( + delta: Omit, +): MonolithDelta { const isCurrentMonolith = delta.headLines >= 400 || delta.baseLines >= 400; - const severity: MonolithSeverity = !isCurrentMonolith || delta.delta <= 0 - ? "none" - : delta.delta >= 20 - ? "blocker" - : "warning"; + const severity: MonolithSeverity = + !isCurrentMonolith || delta.delta <= 0 ? "none" : delta.delta >= 20 ? "blocker" : "warning"; const rationale = !isCurrentMonolith ? "Changed TypeScript file is not a current large-file hotspot." : delta.delta <= 0 @@ -569,12 +662,19 @@ function severityRank(severity: MonolithSeverity): number { function collectDriftEvidence(baseRef: string, changedFiles: string[]): DriftEvidence[] { return changedFiles.slice(0, 50).map((file) => { - const recentHistory = (gitOutput([["log", "--oneline", "--follow", "-20", baseRef, "--", file]], 20000) || "") + const recentHistory = ( + gitOutput([["log", "--oneline", "--follow", "-20", baseRef, "--", file]], 20000) || "" + ) .split("\n") .map((line) => line.trim()) .filter(Boolean); const normalizedFile = file.replace(/^\.\//, "").replace(/\\/g, "/"); - const renameHints = (gitOutput([["log", "--oneline", "--name-status", "--find-renames", "-40", baseRef, "--"]], 120000) || "") + const renameHints = ( + gitOutput( + [["log", "--oneline", "--name-status", "--find-renames", "-40", baseRef, "--"]], + 120000, + ) || "" + ) .split("\n") .map((line) => line.trim()) .filter((line) => { @@ -594,7 +694,10 @@ function countLines(text: string): number { async function collectGitHubContext(): Promise { const repo = process.env.GITHUB_REPOSITORY; - const prNumber = Number.parseInt(process.env.PR_NUMBER || process.env.GITHUB_REF_NAME?.match(/^(\d+)\//)?.[1] || "", 10); + const prNumber = Number.parseInt( + process.env.PR_NUMBER || process.env.GITHUB_REF_NAME?.match(/^(\d+)\//)?.[1] || "", + 10, + ); const token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; if (!repo || !Number.isFinite(prNumber) || prNumber <= 0 || !token) return null; @@ -603,7 +706,11 @@ async function collectGitHubContext(): Promise { const [pullRequest, issueComments, openPulls] = await Promise.all([ githubRest(`repos/${repo}/pulls/${prNumber}`, token), githubRestPaginated(`repos/${repo}/issues/${prNumber}/comments`, token, 100), - githubRestPaginated(`repos/${repo}/pulls?state=open&sort=updated&direction=desc`, token, 100), + githubRestPaginated( + `repos/${repo}/pulls?state=open&sort=updated&direction=desc`, + token, + 100, + ), ]); context.pullRequest = pullRequest; context.previousAdvisorReview = extractPreviousAdvisorReview(issueComments); @@ -611,17 +718,31 @@ async function collectGitHubContext(): Promise { stringOrUndefined(getPath(pullRequest, ["title"])), stringOrUndefined(getPath(pullRequest, ["body"])), stringOrUndefined(getPath(pullRequest, ["head", "ref"])), - ].filter(Boolean).join("\n"); + ] + .filter(Boolean) + .join("\n"); const issueNumbers = extractIssueRefs(prText, prNumber).slice(0, 5); - context.linkedIssues = await Promise.all(issueNumbers.map((issue) => collectLinkedIssue(repo, issue, token))); - context.openPrOverlaps = await collectOpenPrOverlaps(repo, prNumber, token, openPulls, issueNumbers); + context.linkedIssues = await Promise.all( + issueNumbers.map((issue) => collectLinkedIssue(repo, issue, token)), + ); + context.openPrOverlaps = await collectOpenPrOverlaps( + repo, + prNumber, + token, + openPulls, + issueNumbers, + ); } catch (error: unknown) { context.fetchError = error instanceof Error ? error.message : String(error); } return context; } -async function collectLinkedIssue(repo: string, number: number, token: string): Promise { +async function collectLinkedIssue( + repo: string, + number: number, + token: string, +): Promise { try { const [issue, comments] = await Promise.all([ githubRest(`repos/${repo}/issues/${number}`, token), @@ -633,6 +754,25 @@ async function collectLinkedIssue(repo: string, number: number, token: string): } } +async function mapWithConcurrency( + items: readonly T[], + concurrency: number, + mapper: (item: T, index: number) => Promise, +): Promise { + const results = new Array(items.length); + let nextIndex = 0; + const workerCount = Math.min(Math.max(1, concurrency), items.length); + const workers = Array.from({ length: workerCount }, async () => { + while (nextIndex < items.length) { + const index = nextIndex; + nextIndex += 1; + results[index] = await mapper(items[index] as T, index); + } + }); + await Promise.all(workers); + return results; +} + async function collectOpenPrOverlaps( repo: string, currentPrNumber: number, @@ -640,24 +780,45 @@ async function collectOpenPrOverlaps( openPulls: unknown[], currentLinkedIssues: number[], ): Promise { - const currentFiles = new Set((await githubRestPaginated<{ filename?: string }>(`repos/${repo}/pulls/${currentPrNumber}/files`, token, 300)) - .map((file) => file.filename) - .filter((file): file is string => typeof file === "string")); - const overlaps = await Promise.all(openPulls + const currentFiles = new Set( + ( + await githubRestPaginated<{ filename?: string }>( + `repos/${repo}/pulls/${currentPrNumber}/files`, + token, + 300, + ) + ) + .map((file) => file.filename) + .filter((file): file is string => typeof file === "string"), + ); + const candidatePulls = openPulls .filter((pull) => getPath(pull, ["number"]) !== currentPrNumber) - .slice(0, 80) - .map(async (pull): Promise => { + .slice(0, OPEN_PR_OVERLAP_LIMIT); + const overlaps = await mapWithConcurrency( + candidatePulls, + OPEN_PR_OVERLAP_CONCURRENCY, + async (pull): Promise => { const number = getPath(pull, ["number"]); if (!number) return null; const title = stringOrDefault(getPath(pull, ["title"]), `PR #${number}`); const body = stringOrDefault(getPath(pull, ["body"]), ""); - const labels = recordItems(getPath(pull, ["labels"])).map((label) => stringOrUndefined(label.name)).filter((label): label is string => Boolean(label)); + const labels = recordItems(getPath(pull, ["labels"])) + .map((label) => stringOrUndefined(label.name)) + .filter((label): label is string => Boolean(label)); const linkedIssues = extractIssueRefs(`${title}\n${body}`, number); - const duplicateLinkedIssues = linkedIssues.filter((issue) => currentLinkedIssues.includes(issue)); + const duplicateLinkedIssues = linkedIssues.filter((issue) => + currentLinkedIssues.includes(issue), + ); let sameFiles: string[] = []; if (currentFiles.size > 0) { try { - sameFiles = (await githubRestPaginated<{ filename?: string }>(`repos/${repo}/pulls/${number}/files`, token, 300)) + sameFiles = ( + await githubRestPaginated<{ filename?: string }>( + `repos/${repo}/pulls/${number}/files`, + token, + 300, + ) + ) .map((file) => file.filename) .filter((file): file is string => typeof file === "string" && currentFiles.has(file)); } catch { @@ -666,9 +827,16 @@ async function collectOpenPrOverlaps( } if (sameFiles.length === 0 && duplicateLinkedIssues.length === 0) return null; return { number, title, labels, linkedIssues, sameFiles, duplicateLinkedIssues }; - })); - return overlaps.filter((overlap): overlap is OpenPrOverlap => overlap !== null) - .sort((a, b) => b.sameFiles.length - a.sameFiles.length || b.duplicateLinkedIssues.length - a.duplicateLinkedIssues.length || a.number - b.number) + }, + ); + return overlaps + .filter((overlap): overlap is OpenPrOverlap => overlap !== null) + .sort( + (a, b) => + b.sameFiles.length - a.sameFiles.length || + b.duplicateLinkedIssues.length - a.duplicateLinkedIssues.length || + a.number - b.number, + ) .slice(0, 25); } @@ -691,7 +859,9 @@ function extractIssueRefs(text: string, prNumber: number): number[] { function extractPreviousAdvisorReview(issueComments: unknown[]): PreviousAdvisorReview | null { const bodies = issueComments .map((comment) => stringOrUndefined(getPath(comment, ["body"]))) - .filter((body): body is string => Boolean(body && body.includes(""))); + .filter((body): body is string => + Boolean(body && body.includes("")), + ); const body = bodies.at(-1); if (!body) return null; const headSha = body.match(/(?:\*\*Analyzed HEAD:\*\*|Analyzed SHA:)\s*`?([^`\n\s]+)`?/)?.[1]; @@ -703,17 +873,21 @@ export function readTrustedSecurityReviewSkill(): string { return fs.readFileSync(TRUSTED_SECURITY_REVIEW_SKILL_PATH, "utf8"); } catch (error: unknown) { const reason = error instanceof Error ? error.message : String(error); - console.error(`Security review skill unavailable at ${TRUSTED_SECURITY_REVIEW_SKILL_PATH}: ${reason}`); + console.error( + `Security review skill unavailable at ${TRUSTED_SECURITY_REVIEW_SKILL_PATH}: ${reason}`, + ); return ""; } } export function buildSystemPrompt(): string { const securityReviewSkill = readTrustedSecurityReviewSkill(); - const securityRubric = securityReviewSkill || [ - "Trusted security review skill was unavailable; use this built-in 9-category security rubric instead:", - ...SECURITY_CATEGORIES.map((category, index) => `${index + 1}. ${category}`), - ].join("\n"); + const securityRubric = + securityReviewSkill || + [ + "Trusted security review skill was unavailable; use this built-in 9-category security rubric instead:", + ...SECURITY_CATEGORIES.map((category, index) => `${index + 1}. ${category}`), + ].join("\n"); return [ "You are the NemoClaw PR Review Advisor for GitHub Actions.", "NemoClaw runs OpenClaw assistants inside OpenShell sandboxes. Security boundaries, workflows, credentials, network policy, SSRF validation, Dockerfiles, installers, and sandbox lifecycle code are high risk.", @@ -753,7 +927,11 @@ export function buildPromptTurns({ const metadataFields = exactMetadataFields(metadata); const driftContext = JSON.stringify(buildDriftTurnContext(metadata.deterministic), null, 2); const securityContext = JSON.stringify(buildSecurityTurnContext(metadata.deterministic), null, 2); - const validationContext = JSON.stringify(buildValidationTurnContext(metadata.deterministic), null, 2); + const validationContext = JSON.stringify( + buildValidationTurnContext(metadata.deterministic), + null, + 2, + ); return [ { name: "orient-drift", @@ -809,7 +987,10 @@ ${fencedBlock(JSON.stringify(schema), "json")} } function fencedBlock(content: string, language = ""): string { - const longestBacktickRun = Math.max(0, ...Array.from(content.matchAll(/`+/g), (match) => match[0]?.length ?? 0)); + const longestBacktickRun = Math.max( + 0, + ...Array.from(content.matchAll(/`+/g), (match) => match[0]?.length ?? 0), + ); const fence = "`".repeat(Math.max(3, longestBacktickRun + 1)); return `${fence}${language}\n${content}\n${fence}`; } @@ -869,7 +1050,13 @@ export function writePromptArtifacts({ } function promptArtifactSlug(name: string): string { - return name.toLowerCase().replace(/\s+/g, "-").replace(/[^a-z0-9._-]/g, "").slice(0, 80) || "turn"; + return ( + name + .toLowerCase() + .replace(/\s+/g, "-") + .replace(/[^a-z0-9._-]/g, "") + .slice(0, 80) || "turn" + ); } function exactMetadataFields(metadata: ReviewMetadata): string { @@ -882,7 +1069,10 @@ function exactMetadataFields(metadata: ReviewMetadata): string { ].join("\n"); } -export function normalizeReviewResult(result: unknown, metadata: ReviewMetadata): ReviewAdvisorResult { +export function normalizeReviewResult( + result: unknown, + metadata: ReviewMetadata, +): ReviewAdvisorResult { if (!isRecord(result)) throw new Error("PR review advisor returned a non-object result"); const object = result as Record; const sourceOfTruthReview = sanitizeSourceOfTruthReview(object.sourceOfTruthReview); @@ -913,12 +1103,17 @@ function sanitizeSummary(value: unknown): ReviewAdvisorResult["summary"] { recommendation: enumValue(object.recommendation, SUMMARY_RECOMMENDATIONS, "info_only"), confidence: enumValue(object.confidence, CONFIDENCES, "medium"), oneLine: stringOrDefault(object.oneLine, "PR review advisor completed with limited summary."), - topItem: typeof object.topItem === "string" && object.topItem.trim() ? object.topItem.trim() : undefined, + topItem: + typeof object.topItem === "string" && object.topItem.trim() + ? object.topItem.trim() + : undefined, sinceLastReview: sanitizeSinceLastReview(object.sinceLastReview), }; } -function sanitizeSinceLastReview(value: unknown): ReviewAdvisorResult["summary"]["sinceLastReview"] { +function sanitizeSinceLastReview( + value: unknown, +): ReviewAdvisorResult["summary"]["sinceLastReview"] { if (!isRecord(value)) return undefined; return { resolved: nonNegativeInteger(value.resolved), @@ -932,24 +1127,35 @@ function nonNegativeInteger(value: unknown): number { } function sanitizeFindings(value: unknown): Finding[] { - return recordItems(value).map((item) => ({ - severity: enumValue(item.severity, ["blocker", "warning", "suggestion"] as const, "suggestion"), - category: enumValue(item.category, FINDING_CATEGORIES, "correctness"), - file: typeof item.file === "string" ? item.file : null, - line: typeof item.line === "number" && Number.isInteger(item.line) && item.line > 0 ? item.line : null, - title: stringOrDefault(item.title, "Review finding"), - description: stringOrDefault(item.description, "No description provided."), - recommendation: stringOrDefault(item.recommendation, "Review manually."), - evidence: stringOrDefault(item.evidence, "No evidence provided."), - })).slice(0, 50); + return recordItems(value) + .map((item) => ({ + severity: enumValue( + item.severity, + ["blocker", "warning", "suggestion"] as const, + "suggestion", + ), + category: enumValue(item.category, FINDING_CATEGORIES, "correctness"), + file: typeof item.file === "string" ? item.file : null, + line: + typeof item.line === "number" && Number.isInteger(item.line) && item.line > 0 + ? item.line + : null, + title: stringOrDefault(item.title, "Review finding"), + description: stringOrDefault(item.description, "No description provided."), + recommendation: stringOrDefault(item.recommendation, "Review manually."), + evidence: stringOrDefault(item.evidence, "No evidence provided."), + })) + .slice(0, 50); } function sanitizeAcceptanceCoverage(value: unknown): AcceptanceCoverage[] { - return recordItems(value).map((item) => ({ - clause: stringOrDefault(item.clause, "Unspecified acceptance clause"), - status: enumValue(item.status, ACCEPTANCE_STATUSES, "unknown"), - evidence: stringOrDefault(item.evidence, "No evidence provided."), - })).slice(0, 100); + return recordItems(value) + .map((item) => ({ + clause: stringOrDefault(item.clause, "Unspecified acceptance clause"), + status: enumValue(item.status, ACCEPTANCE_STATUSES, "unknown"), + evidence: stringOrDefault(item.evidence, "No evidence provided."), + })) + .slice(0, 100); } function sanitizeSecurityCategories(value: unknown): SecurityCategory[] { @@ -967,24 +1173,31 @@ function sanitizeSecurityCategories(value: unknown): SecurityCategory[] { } function sanitizeSourceOfTruthReview(value: unknown): SourceOfTruthReview[] { - return recordItems(value).map((item) => ({ - surface: stringOrDefault(item.surface, "Unspecified localized patch surface"), - status: enumValue(item.status, SOURCE_OF_TRUTH_STATUSES, "not_applicable"), - invalidState: stringOrDefault(item.invalidState, "Not specified."), - sourceBoundary: stringOrDefault(item.sourceBoundary, "Not specified."), - whyNotSourceFix: stringOrDefault(item.whyNotSourceFix, "Not specified."), - regressionTest: stringOrDefault(item.regressionTest, "Not specified."), - removalCondition: stringOrDefault(item.removalCondition, "Not specified."), - evidence: stringOrDefault(item.evidence, "No evidence provided."), - })).slice(0, 50); + return recordItems(value) + .map((item) => ({ + surface: stringOrDefault(item.surface, "Unspecified localized patch surface"), + status: enumValue(item.status, SOURCE_OF_TRUTH_STATUSES, "not_applicable"), + invalidState: stringOrDefault(item.invalidState, "Not specified."), + sourceBoundary: stringOrDefault(item.sourceBoundary, "Not specified."), + whyNotSourceFix: stringOrDefault(item.whyNotSourceFix, "Not specified."), + regressionTest: stringOrDefault(item.regressionTest, "Not specified."), + removalCondition: stringOrDefault(item.removalCondition, "Not specified."), + evidence: stringOrDefault(item.evidence, "No evidence provided."), + })) + .slice(0, 50); } -function addSourceOfTruthFindings(findings: Finding[], sourceOfTruthReview: SourceOfTruthReview[]): Finding[] { +function addSourceOfTruthFindings( + findings: Finding[], + sourceOfTruthReview: SourceOfTruthReview[], +): Finding[] { const injected: Finding[] = []; for (const review of sourceOfTruthReview) { if (review.status !== "missing" && review.status !== "needs_followup") continue; const alreadyCovered = [...injected, ...findings].some((finding) => - `${finding.title}\n${finding.description}\n${finding.evidence}`.toLowerCase().includes(review.surface.toLowerCase()), + `${finding.title}\n${finding.description}\n${finding.evidence}` + .toLowerCase() + .includes(review.surface.toLowerCase()), ); if (alreadyCovered) continue; injected.push({ @@ -994,7 +1207,8 @@ function addSourceOfTruthFindings(findings: Finding[], sourceOfTruthReview: Sour line: null, title: `Source-of-truth review needed: ${review.surface}`, description: `The advisor marked localized patch analysis as ${review.status}.`, - recommendation: "Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.", + recommendation: + "Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.", evidence: review.evidence, }); } @@ -1021,7 +1235,10 @@ function addDeterministicFindings(findings: Finding[], metadata: ReviewMetadata) return [...injected, ...findings.slice(0, originalSlots)]; } -function sanitizeTestDepth(value: unknown, fallback: ReviewAdvisorResult["testDepth"]): ReviewAdvisorResult["testDepth"] { +function sanitizeTestDepth( + value: unknown, + fallback: ReviewAdvisorResult["testDepth"], +): ReviewAdvisorResult["testDepth"] { const object = isRecord(value) ? value : {}; return { verdict: enumValue(object.verdict, TEST_DEPTH_VERDICTS, fallback.verdict), @@ -1034,8 +1251,12 @@ function sanitizeReviewCompleteness(value: unknown): ReviewAdvisorResult["review const object = isRecord(value) ? value : {}; const limitations = stringArray(object.limitations); return { - limitations: limitations.length > 0 ? limitations : ["Automated review only; human maintainer review is required before merge."], - requiresHumanReview: typeof object.requiresHumanReview === "boolean" ? object.requiresHumanReview : true, + limitations: + limitations.length > 0 + ? limitations + : ["Automated review only; human maintainer review is required before merge."], + requiresHumanReview: + typeof object.requiresHumanReview === "boolean" ? object.requiresHumanReview : true, }; } @@ -1109,17 +1330,27 @@ function collectTestingFollowups(result: ReviewAdvisorResult): string[] { const followups: string[] = []; if (result.testDepth.verdict !== "unit_sufficient") { for (const suggestion of result.testDepth.suggestedTests.slice(0, 5)) { - followups.push(`**${testDepthLabel(result.testDepth.verdict)}** — ${suggestion}. ${result.testDepth.rationale}`); + followups.push( + `**${testDepthLabel(result.testDepth.verdict)}** — ${suggestion}. ${result.testDepth.rationale}`, + ); } } for (const finding of result.findings.filter((item) => item.category === "tests").slice(0, 5)) { followups.push(`**${finding.title}** — ${finding.recommendation}`); } - for (const clause of result.acceptanceCoverage.filter((item) => item.status !== "met").slice(0, 5)) { - followups.push(`**Acceptance clause:** ${clause.clause} — add test evidence or identify existing coverage. ${clause.evidence}`); + for (const clause of result.acceptanceCoverage + .filter((item) => item.status !== "met") + .slice(0, 5)) { + followups.push( + `**Acceptance clause:** ${clause.clause} — add test evidence or identify existing coverage. ${clause.evidence}`, + ); } - for (const review of result.sourceOfTruthReview.filter((item) => item.status === "missing" || item.status === "needs_followup").slice(0, 5)) { - followups.push(`**${review.surface}** — ${review.regressionTest || "add a regression test for the localized behavior"}. ${review.evidence}`); + for (const review of result.sourceOfTruthReview + .filter((item) => item.status === "missing" || item.status === "needs_followup") + .slice(0, 5)) { + followups.push( + `**${review.surface}** — ${review.regressionTest || "add a regression test for the localized behavior"}. ${review.evidence}`, + ); } return [...new Set(followups)].slice(0, 8); } @@ -1136,7 +1367,9 @@ function appendFindings(lines: string[], heading: string, findings: Finding[]): lines.push("- _None._"); } else { for (const finding of findings.slice(0, 20)) { - const location = finding.file ? ` (${finding.file}${finding.line ? `:${finding.line}` : ""})` : ""; + const location = finding.file + ? ` (${finding.file}${finding.line ? `:${finding.line}` : ""})` + : ""; lines.push(`- **${finding.title}**${location}: ${finding.description}`); lines.push(` - Recommendation: ${finding.recommendation}`); lines.push(` - Evidence: ${finding.evidence}`); @@ -1145,7 +1378,11 @@ function appendFindings(lines: string[], heading: string, findings: Finding[]): lines.push(""); } -function unavailableResult(metadata: ReviewMetadata, reason: string, failed: boolean): ReviewAdvisorResult { +function unavailableResult( + metadata: ReviewMetadata, + reason: string, + failed: boolean, +): ReviewAdvisorResult { return { version: 1, baseRef: metadata.baseRef, @@ -1155,19 +1392,23 @@ function unavailableResult(metadata: ReviewMetadata, reason: string, failed: boo summary: { recommendation: "info_only", confidence: "low", - oneLine: failed ? `PR review advisor failed: ${reason}` : `PR review advisor skipped: ${reason}`, + oneLine: failed + ? `PR review advisor failed: ${reason}` + : `PR review advisor skipped: ${reason}`, }, findings: failed - ? [{ - severity: "warning", - category: "correctness", - file: null, - line: null, - title: "PR review advisor unavailable", - description: `The automated advisor could not complete: ${reason}`, - recommendation: "Re-run the PR Review Advisor or perform a manual review.", - evidence: reason, - }] + ? [ + { + severity: "warning", + category: "correctness", + file: null, + line: null, + title: "PR review advisor unavailable", + description: `The automated advisor could not complete: ${reason}`, + recommendation: "Re-run the PR Review Advisor or perform a manual review.", + evidence: reason, + }, + ] : [], acceptanceCoverage: [], securityCategories: SECURITY_CATEGORIES.map((category) => ({ @@ -1179,7 +1420,9 @@ function unavailableResult(metadata: ReviewMetadata, reason: string, failed: boo testDepth: metadata.deterministic.testDepth, positives: [], reviewCompleteness: { - limitations: [failed ? `Advisor execution failed: ${reason}` : `Advisor execution skipped: ${reason}`], + limitations: [ + failed ? `Advisor execution failed: ${reason}` : `Advisor execution skipped: ${reason}`, + ], requiresHumanReview: true, }, }; diff --git a/tools/pr-review-advisor/comment.mts b/tools/pr-review-advisor/comment.mts index c279e5ed5d..ca9fcdc6db 100755 --- a/tools/pr-review-advisor/comment.mts +++ b/tools/pr-review-advisor/comment.mts @@ -65,11 +65,13 @@ async function main(): Promise { const repo = args.repo || process.env.GITHUB_REPOSITORY; const pr = args.pr || process.env.PR_NUMBER; const summaryPath = args.summary || "artifacts/pr-review-advisor/pr-review-advisor-summary.md"; - const resultPath = args.result || "artifacts/pr-review-advisor/pr-review-advisor-final-result.json"; + const resultPath = + args.result || "artifacts/pr-review-advisor/pr-review-advisor-final-result.json"; const token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; - const runUrl = process.env.GITHUB_SERVER_URL && process.env.GITHUB_REPOSITORY && process.env.GITHUB_RUN_ID - ? `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}` - : undefined; + const runUrl = + process.env.GITHUB_SERVER_URL && process.env.GITHUB_REPOSITORY && process.env.GITHUB_RUN_ID + ? `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}` + : undefined; if (!repo || !pr) { console.log("Skipping PR review advisor comment: repo or PR number not provided"); @@ -80,7 +82,9 @@ async function main(): Promise { return; } - const summary = readIfExists(summaryPath) || readIfExists("artifacts/pr-review-advisor/pr-review-advisor-summary.md"); + const summary = + readIfExists(summaryPath) || + readIfExists("artifacts/pr-review-advisor/pr-review-advisor-summary.md"); if (!summary) throw new Error(`No PR review advisor summary found at ${summaryPath}`); const result = readJsonIfExists(resultPath); const body = buildComment({ summary, result, runUrl, marker: MARKER }); @@ -99,16 +103,17 @@ export function buildComment({ runUrl?: string; marker?: string; }): string { - const blockerCount = result?.findings?.filter((finding) => finding.severity === "blocker").length ?? 0; - const warningCount = result?.findings?.filter((finding) => finding.severity === "warning").length ?? 0; - const suggestionCount = result?.findings?.filter((finding) => finding.severity === "suggestion").length ?? 0; + const blockerCount = + result?.findings?.filter((finding) => finding.severity === "blocker").length ?? 0; + const warningCount = + result?.findings?.filter((finding) => finding.severity === "warning").length ?? 0; + const suggestionCount = + result?.findings?.filter((finding) => finding.severity === "suggestion").length ?? 0; const secondary = buildSecondarySummary(result); const findingsDetails = renderFindingsDetails(result); const testingFollowupsDetails = renderTestingFollowupsDetails(result); const previousReviewDetails = renderPreviousReviewDetails(result); - const details = runUrl - ? `\n[Workflow run details](${runUrl})` - : ""; + const details = runUrl ? `\n[Workflow run details](${runUrl})` : ""; return `${marker || MARKER} ## PR Review Advisor @@ -130,17 +135,28 @@ function buildSecondarySummary(result?: ReviewAdvisorResult): string { } function topFindingTitle(result?: ReviewAdvisorResult): string | undefined { - return result?.findings?.find((finding) => finding.severity === "blocker")?.title || + return ( + result?.findings?.find((finding) => finding.severity === "blocker")?.title || result?.findings?.find((finding) => finding.severity === "warning")?.title || - result?.findings?.find((finding) => finding.severity === "suggestion")?.title; + result?.findings?.find((finding) => finding.severity === "suggestion")?.title + ); } function renderFindingsDetails(result?: ReviewAdvisorResult): string { if (!result?.findings?.length) return ""; const sections = [ - { summary: "🛠️ Needs attention", findings: result.findings.filter((finding) => finding.severity === "blocker") }, - { summary: "🔎 Worth checking", findings: result.findings.filter((finding) => finding.severity === "warning") }, - { summary: "🌱 Nice ideas", findings: result.findings.filter((finding) => finding.severity === "suggestion") }, + { + summary: "🛠️ Needs attention", + findings: result.findings.filter((finding) => finding.severity === "blocker"), + }, + { + summary: "🔎 Worth checking", + findings: result.findings.filter((finding) => finding.severity === "warning"), + }, + { + summary: "🌱 Nice ideas", + findings: result.findings.filter((finding) => finding.severity === "suggestion"), + }, ]; const lines: string[] = ["", "
", "Review findings", ""]; for (const section of sections) { @@ -161,7 +177,12 @@ function renderFindingsDetails(result?: ReviewAdvisorResult): string { function renderTestingFollowupsDetails(result?: ReviewAdvisorResult): string { const followups = collectTestingFollowups(result); if (followups.length === 0) return ""; - const lines: string[] = ["", "
", "Consider writing more tests for", ""]; + const lines: string[] = [ + "", + "
", + "Consider writing more tests for", + "", + ]; for (const followup of followups) lines.push(`- ${escapeCommentText(followup)}`); lines.push("", "
", ""); return `${lines.join("\n")}\n`; @@ -177,14 +198,25 @@ function collectTestingFollowups(result?: ReviewAdvisorResult): string[] { followups.push(`**${label}** — ${suggestion}.${rationale}`); } } - for (const finding of result.findings?.filter((item) => item.category === "tests").slice(0, 5) || []) { - followups.push(`**${finding.title || "Test coverage"}** — ${finding.recommendation || finding.description || "Add targeted coverage for the changed behavior."}`); + for (const finding of result.findings?.filter((item) => item.category === "tests").slice(0, 5) || + []) { + followups.push( + `**${finding.title || "Test coverage"}** — ${finding.recommendation || finding.description || "Add targeted coverage for the changed behavior."}`, + ); } - for (const clause of result.acceptanceCoverage?.filter((item) => item.status && item.status !== "met").slice(0, 5) || []) { - followups.push(`**Acceptance clause:** ${clause.clause || "unspecified"} — add test evidence or identify existing coverage. ${clause.evidence || ""}`.trim()); + for (const clause of result.acceptanceCoverage + ?.filter((item) => item.status && item.status !== "met") + .slice(0, 5) || []) { + followups.push( + `**Acceptance clause:** ${clause.clause || "unspecified"} — add test evidence or identify existing coverage. ${clause.evidence || ""}`.trim(), + ); } - for (const review of result.sourceOfTruthReview?.filter((item) => item.status === "missing" || item.status === "needs_followup").slice(0, 5) || []) { - followups.push(`**${review.surface || "Localized behavior"}** — ${review.regressionTest || "add a regression test for the localized behavior"}. ${review.evidence || ""}`.trim()); + for (const review of result.sourceOfTruthReview + ?.filter((item) => item.status === "missing" || item.status === "needs_followup") + .slice(0, 5) || []) { + followups.push( + `**${review.surface || "Localized behavior"}** — ${review.regressionTest || "add a regression test for the localized behavior"}. ${review.evidence || ""}`.trim(), + ); } return [...new Set(followups)].slice(0, 8); } @@ -217,7 +249,9 @@ function formatFinding(finding: NonNullable[num return lines.join("\n"); } -function formatFindingLocation(finding: NonNullable[number]): string { +function formatFindingLocation( + finding: NonNullable[number], +): string { if (!finding.file) return ""; const line = Number.isInteger(finding.line) && Number(finding.line) > 0 ? `:${finding.line}` : ""; return ` (${escapeCommentText(finding.file)}${line})`; diff --git a/tools/pr-review-advisor/workflow-boundary.mts b/tools/pr-review-advisor/workflow-boundary.mts index c0e11dd358..cec27d4821 100644 --- a/tools/pr-review-advisor/workflow-boundary.mts +++ b/tools/pr-review-advisor/workflow-boundary.mts @@ -25,7 +25,9 @@ function asRecord(value: unknown): WorkflowRecord { } function asSteps(value: unknown): WorkflowStep[] { - return Array.isArray(value) ? (value.filter((entry) => asRecord(entry) === entry) as WorkflowStep[]) : []; + return Array.isArray(value) + ? (value.filter((entry) => asRecord(entry) === entry) as WorkflowStep[]) + : []; } function stringValue(value: unknown): string { @@ -123,7 +125,11 @@ export function validatePrReviewAdvisorWorkflowBoundary( errors.push("PR checkout must use the pull request head SHA as inert analysis data"); } - const dispatchCheckout = requireStep(errors, steps, "Checkout dispatch workspace (read-only data)"); + const dispatchCheckout = requireStep( + errors, + steps, + "Checkout dispatch workspace (read-only data)", + ); requireStepWith(errors, dispatchCheckout, "path", "pr-workdir"); requireStepWith(errors, dispatchCheckout, "persist-credentials", false); @@ -132,7 +138,7 @@ export function validatePrReviewAdvisorWorkflowBoundary( requireRunContains(errors, install, "$ADVISOR_DIR/node_modules"); const analyze = requireStep(errors, steps, "Run PR review advisor"); - requireRunContains(errors, analyze, "cd \"$ADVISOR_WORKDIR\""); + requireRunContains(errors, analyze, 'cd "$ADVISOR_WORKDIR"'); requireRunContains(errors, analyze, "$ADVISOR_DIR/tools/pr-review-advisor/analyze.mts"); requireRunContains(errors, analyze, "$ADVISOR_DIR/tools/pr-review-advisor/schema.json");