From 2fc9fc78920f1ab8c4c2f5a03dcdc3ecd1a2630d Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Tue, 14 Apr 2026 13:22:38 -0400 Subject: [PATCH] QA: harden runner metadata and install hints Move optional QA runner install hints onto a generated metadata catalog so the host no longer needs a Matrix-specific fallback list for missing plugins. This also tightens the runner contract by rejecting runtime-only commands that are not declared in manifest metadata, and adds an installed-plugin smoke test for the generic QA runner loader path. --- extensions/qa-lab/src/cli.test.ts | 10 +- extensions/qa-lab/src/live-transports/cli.ts | 35 ++--- package.json | 2 + scripts/generate-qa-runner-catalog.ts | 35 +++++ scripts/lib/qa-runner-catalog.json | 8 + .../qa-runner-runtime.integration.test.ts | 143 ++++++++++++++++++ src/plugin-sdk/qa-runner-runtime.test.ts | 54 +++++++ src/plugin-sdk/qa-runner-runtime.ts | 58 +++++-- src/plugins/qa-runner-catalog.ts | 74 +++++++++ 9 files changed, 387 insertions(+), 32 deletions(-) create mode 100644 scripts/generate-qa-runner-catalog.ts create mode 100644 scripts/lib/qa-runner-catalog.json create mode 100644 src/plugin-sdk/qa-runner-runtime.integration.test.ts create mode 100644 src/plugins/qa-runner-catalog.ts diff --git a/extensions/qa-lab/src/cli.test.ts b/extensions/qa-lab/src/cli.test.ts index afd802ee99a..182ea67a53d 100644 --- a/extensions/qa-lab/src/cli.test.ts +++ b/extensions/qa-lab/src/cli.test.ts @@ -88,7 +88,15 @@ describe("qa cli registration", () => { }); it("shows an install hint when the matrix runner plugin is unavailable", async () => { - listQaRunnerCliContributions.mockReset().mockReturnValue([]); + listQaRunnerCliContributions.mockReset().mockReturnValue([ + { + pluginId: "qa-matrix", + commandName: "matrix", + description: "Run the Matrix live QA lane", + status: "missing", + npmSpec: "@openclaw/qa-matrix", + }, + ]); const missingProgram = new Command(); registerQaLabCli(missingProgram); diff --git a/extensions/qa-lab/src/live-transports/cli.ts b/extensions/qa-lab/src/live-transports/cli.ts index 1b2feb59d72..ea2c1b7b6ef 100644 --- a/extensions/qa-lab/src/live-transports/cli.ts +++ b/extensions/qa-lab/src/live-transports/cli.ts @@ -2,14 +2,6 @@ import { listQaRunnerCliContributions } from "openclaw/plugin-sdk/qa-runner-runt import type { LiveTransportQaCliRegistration } from "./shared/live-transport-cli.js"; import { telegramQaCliRegistration } from "./telegram/cli.js"; -const OPTIONAL_QA_RUNNER_INSTALLS = [ - { - commandName: "matrix", - description: "Run the Matrix live QA lane (install @openclaw/qa-matrix first)", - npmSpec: "@openclaw/qa-matrix", - }, -] as const; - function createMissingQaRunnerCliRegistration(params: { commandName: string; description: string; @@ -55,27 +47,26 @@ export const LIVE_TRANSPORT_QA_CLI_REGISTRATIONS: readonly LiveTransportQaCliReg export function listLiveTransportQaCliRegistrations(): readonly LiveTransportQaCliRegistration[] { const liveRegistrations = [...LIVE_TRANSPORT_QA_CLI_REGISTRATIONS]; const discoveredRunners = listQaRunnerCliContributions(); - const seenCommandNames = new Set(liveRegistrations.map((registration) => registration.commandName)); for (const runner of discoveredRunners) { - seenCommandNames.add(runner.commandName); liveRegistrations.push( runner.status === "available" ? runner.registration - : createBlockedQaRunnerCliRegistration({ - commandName: runner.commandName, - description: runner.description, - pluginId: runner.pluginId, - }), + : runner.status === "blocked" + ? createBlockedQaRunnerCliRegistration({ + commandName: runner.commandName, + description: runner.description, + pluginId: runner.pluginId, + }) + : createMissingQaRunnerCliRegistration({ + commandName: runner.commandName, + description: + runner.description ?? + `Run the ${runner.commandName} live QA lane (install ${runner.npmSpec} first)`, + npmSpec: runner.npmSpec, + }), ); } - for (const runner of OPTIONAL_QA_RUNNER_INSTALLS) { - if (seenCommandNames.has(runner.commandName)) { - continue; - } - liveRegistrations.push(createMissingQaRunnerCliRegistration(runner)); - } - return liveRegistrations; } diff --git a/package.json b/package.json index aeebb76f4a5..85b44b51948 100644 --- a/package.json +++ b/package.json @@ -1230,6 +1230,8 @@ "plugin-sdk:sync-exports": "node scripts/sync-plugin-sdk-exports.mjs", "plugin-sdk:usage": "node --import tsx scripts/analyze-plugin-sdk-usage.ts", "plugins:sync": "node --import tsx scripts/sync-plugin-versions.ts", + "qa-runners:check": "node --import tsx scripts/generate-qa-runner-catalog.ts --check", + "qa-runners:gen": "node --import tsx scripts/generate-qa-runner-catalog.ts --write", "postinstall": "node scripts/postinstall-bundled-plugins.mjs", "prepack": "node --import tsx scripts/openclaw-prepack.ts", "prepare": "command -v git >/dev/null 2>&1 && git rev-parse --is-inside-work-tree >/dev/null 2>&1 && git config core.hooksPath git-hooks || exit 0", diff --git a/scripts/generate-qa-runner-catalog.ts b/scripts/generate-qa-runner-catalog.ts new file mode 100644 index 00000000000..a49201628bc --- /dev/null +++ b/scripts/generate-qa-runner-catalog.ts @@ -0,0 +1,35 @@ +#!/usr/bin/env node +import path from "node:path"; +import { writeBundledQaRunnerCatalog } from "../src/plugins/qa-runner-catalog.js"; + +const args = new Set(process.argv.slice(2)); +const checkOnly = args.has("--check"); +const writeMode = args.has("--write"); + +if (checkOnly === writeMode) { + console.error("Use exactly one of --check or --write."); + process.exit(1); +} + +const repoRoot = process.cwd(); +const result = await writeBundledQaRunnerCatalog({ + repoRoot, + check: checkOnly, +}); + +if (checkOnly) { + if (result.changed) { + console.error( + [ + "QA runner catalog drift detected.", + `Expected current: ${path.relative(repoRoot, result.jsonPath)}`, + "If this QA runner metadata change is intentional, run `pnpm qa-runners:gen` and commit the updated baseline file.", + "If not intentional, fix the bundled plugin metadata drift first.", + ].join("\n"), + ); + process.exit(1); + } + console.log(`OK ${path.relative(repoRoot, result.jsonPath)}`); +} else { + console.log(`Wrote ${path.relative(repoRoot, result.jsonPath)}`); +} diff --git a/scripts/lib/qa-runner-catalog.json b/scripts/lib/qa-runner-catalog.json new file mode 100644 index 00000000000..06864acd78b --- /dev/null +++ b/scripts/lib/qa-runner-catalog.json @@ -0,0 +1,8 @@ +[ + { + "pluginId": "qa-matrix", + "commandName": "matrix", + "description": "Run the Docker-backed Matrix live QA lane against a disposable homeserver", + "npmSpec": "@openclaw/qa-matrix" + } +] diff --git a/src/plugin-sdk/qa-runner-runtime.integration.test.ts b/src/plugin-sdk/qa-runner-runtime.integration.test.ts new file mode 100644 index 00000000000..1ad93b1e7b5 --- /dev/null +++ b/src/plugin-sdk/qa-runner-runtime.integration.test.ts @@ -0,0 +1,143 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { clearPluginDiscoveryCache } from "../plugins/discovery.js"; +import { clearPluginManifestRegistryCache } from "../plugins/manifest-registry.js"; +import { resetFacadeRuntimeStateForTest } from "./facade-runtime.js"; + +const ORIGINAL_ENV = { + OPENCLAW_DISABLE_BUNDLED_PLUGINS: process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS, + OPENCLAW_CONFIG_PATH: process.env.OPENCLAW_CONFIG_PATH, + OPENCLAW_DISABLE_PLUGIN_DISCOVERY_CACHE: process.env.OPENCLAW_DISABLE_PLUGIN_DISCOVERY_CACHE, + OPENCLAW_DISABLE_PLUGIN_MANIFEST_CACHE: process.env.OPENCLAW_DISABLE_PLUGIN_MANIFEST_CACHE, + OPENCLAW_PLUGIN_DISCOVERY_CACHE_MS: process.env.OPENCLAW_PLUGIN_DISCOVERY_CACHE_MS, + OPENCLAW_PLUGIN_MANIFEST_CACHE_MS: process.env.OPENCLAW_PLUGIN_MANIFEST_CACHE_MS, + OPENCLAW_TEST_FAST: process.env.OPENCLAW_TEST_FAST, +} as const; + +const tempDirs: string[] = []; + +function makeTempDir(prefix: string): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} + +function resetQaRunnerRuntimeState() { + clearPluginDiscoveryCache(); + clearPluginManifestRegistryCache(); + resetFacadeRuntimeStateForTest(); +} + +describe("plugin-sdk qa-runner-runtime linked plugin smoke", () => { + beforeEach(() => { + resetQaRunnerRuntimeState(); + process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS = "1"; + process.env.OPENCLAW_DISABLE_PLUGIN_DISCOVERY_CACHE = "1"; + process.env.OPENCLAW_DISABLE_PLUGIN_MANIFEST_CACHE = "1"; + process.env.OPENCLAW_PLUGIN_DISCOVERY_CACHE_MS = "0"; + process.env.OPENCLAW_PLUGIN_MANIFEST_CACHE_MS = "0"; + process.env.OPENCLAW_TEST_FAST = "1"; + }); + + afterEach(() => { + resetQaRunnerRuntimeState(); + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } + for (const [key, value] of Object.entries(ORIGINAL_ENV)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + }); + + it("loads an activated qa runner from a linked plugin path", async () => { + const stateDir = makeTempDir("openclaw-qa-runner-state-"); + const pluginDir = path.join(stateDir, "extensions", "qa-linked"); + const configPath = path.join(stateDir, "openclaw.json"); + + fs.writeFileSync( + configPath, + JSON.stringify({ + plugins: {}, + }), + "utf8", + ); + process.env.OPENCLAW_CONFIG_PATH = configPath; + + fs.mkdirSync(pluginDir, { recursive: true }); + fs.writeFileSync( + path.join(pluginDir, "openclaw.plugin.json"), + JSON.stringify({ + id: "qa-linked", + qaRunners: [ + { + commandName: "linked", + description: "Run the linked QA lane", + }, + ], + configSchema: { + type: "object", + additionalProperties: false, + properties: {}, + }, + }), + "utf8", + ); + fs.writeFileSync( + path.join(pluginDir, "package.json"), + JSON.stringify({ + name: "@openclaw/qa-linked", + type: "module", + openclaw: { + extensions: ["./index.js"], + install: { + npmSpec: "@openclaw/qa-linked", + }, + }, + }), + "utf8", + ); + fs.writeFileSync(path.join(pluginDir, "index.js"), 'export default {};\n', "utf8"); + fs.writeFileSync( + path.join(pluginDir, "runtime-api.js"), + [ + "export const qaRunnerCliRegistrations = [", + " {", + ' commandName: "linked",', + " register() {}", + " }", + "];", + ].join("\n"), + "utf8", + ); + + const module = await import("./qa-runner-runtime.js"); + + expect(module.listQaRunnerCliContributions()).toEqual( + expect.arrayContaining([ + { + pluginId: "qa-linked", + commandName: "linked", + description: "Run the linked QA lane", + status: "available", + registration: { + commandName: "linked", + register: expect.any(Function), + }, + }, + { + pluginId: "qa-matrix", + commandName: "matrix", + description: "Run the Docker-backed Matrix live QA lane against a disposable homeserver", + status: "missing", + npmSpec: "@openclaw/qa-matrix", + }, + ]), + ); + }); +}); diff --git a/src/plugin-sdk/qa-runner-runtime.test.ts b/src/plugin-sdk/qa-runner-runtime.test.ts index 13cc4f6ee0f..99c0c99f658 100644 --- a/src/plugin-sdk/qa-runner-runtime.test.ts +++ b/src/plugin-sdk/qa-runner-runtime.test.ts @@ -3,11 +3,16 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const loadPluginManifestRegistry = vi.hoisted(() => vi.fn()); const tryLoadActivatedBundledPluginPublicSurfaceModuleSync = vi.hoisted(() => vi.fn()); +const listBundledQaRunnerCatalog = vi.hoisted(() => vi.fn(() => [])); vi.mock("../plugins/manifest-registry.js", () => ({ loadPluginManifestRegistry, })); +vi.mock("../plugins/qa-runner-catalog.js", () => ({ + listBundledQaRunnerCatalog, +})); + vi.mock("./facade-runtime.js", () => ({ tryLoadActivatedBundledPluginPublicSurfaceModuleSync, })); @@ -18,6 +23,7 @@ describe("plugin-sdk qa-runner-runtime", () => { plugins: [], diagnostics: [], }); + listBundledQaRunnerCatalog.mockReset().mockReturnValue([]); tryLoadActivatedBundledPluginPublicSurfaceModuleSync.mockReset(); }); @@ -93,6 +99,29 @@ describe("plugin-sdk qa-runner-runtime", () => { ]); }); + it("reports missing optional runners from the generated catalog", async () => { + listBundledQaRunnerCatalog.mockReturnValue([ + { + pluginId: "qa-matrix", + commandName: "matrix", + description: "Run the Matrix live QA lane", + npmSpec: "@openclaw/qa-matrix", + }, + ]); + + const module = await import("./qa-runner-runtime.js"); + + expect(module.listQaRunnerCliContributions()).toEqual([ + { + pluginId: "qa-matrix", + commandName: "matrix", + description: "Run the Matrix live QA lane", + status: "missing", + npmSpec: "@openclaw/qa-matrix", + }, + ]); + }); + it("fails fast when two plugins declare the same qa runner command", async () => { loadPluginManifestRegistry.mockReturnValue({ plugins: [ @@ -117,4 +146,29 @@ describe("plugin-sdk qa-runner-runtime", () => { 'QA runner command "matrix" declared by both "alpha" and "beta"', ); }); + + it("fails when runtime registrations include an undeclared command", async () => { + loadPluginManifestRegistry.mockReturnValue({ + plugins: [ + { + id: "qa-matrix", + qaRunners: [{ commandName: "matrix" }], + rootDir: "/tmp/qa-matrix", + }, + ], + diagnostics: [], + }); + tryLoadActivatedBundledPluginPublicSurfaceModuleSync.mockReturnValue({ + qaRunnerCliRegistrations: [ + { commandName: "matrix", register: vi.fn() }, + { commandName: "extra", register: vi.fn() }, + ], + }); + + const module = await import("./qa-runner-runtime.js"); + + expect(() => module.listQaRunnerCliContributions()).toThrow( + 'QA runner plugin "qa-matrix" exported "extra" from runtime-api.js but did not declare it in openclaw.plugin.json', + ); + }); }); diff --git a/src/plugin-sdk/qa-runner-runtime.ts b/src/plugin-sdk/qa-runner-runtime.ts index 6b7bf7769ce..e4ce8787e5a 100644 --- a/src/plugin-sdk/qa-runner-runtime.ts +++ b/src/plugin-sdk/qa-runner-runtime.ts @@ -1,6 +1,7 @@ import type { Command } from "commander"; import type { PluginManifestRecord } from "../plugins/manifest-registry.js"; import { loadPluginManifestRegistry } from "../plugins/manifest-registry.js"; +import { listBundledQaRunnerCatalog } from "../plugins/qa-runner-catalog.js"; import { tryLoadActivatedBundledPluginPublicSurfaceModuleSync } from "./facade-runtime.js"; export type QaRunnerCliRegistration = { @@ -26,6 +27,13 @@ export type QaRunnerCliContribution = commandName: string; description?: string; status: "blocked"; + } + | { + pluginId: string; + commandName: string; + description?: string; + status: "missing"; + npmSpec: string; }; function listDeclaredQaRunnerPlugins(): Array< @@ -69,9 +77,33 @@ function listRuntimeRegistrations( return registrations; } -export function listQaRunnerCliContributions(): readonly QaRunnerCliContribution[] { - const contributions: QaRunnerCliContribution[] = []; +function buildKnownQaRunnerCatalog(): readonly QaRunnerCliContribution[] { + const knownRunners = listBundledQaRunnerCatalog(); const seenCommandNames = new Map(); + return knownRunners.map((runner) => { + const previousOwner = seenCommandNames.get(runner.commandName); + if (previousOwner) { + throw new Error( + `QA runner command "${runner.commandName}" declared by both "${previousOwner}" and "${runner.pluginId}"`, + ); + } + seenCommandNames.set(runner.commandName, runner.pluginId); + return { + pluginId: runner.pluginId, + commandName: runner.commandName, + ...(runner.description ? { description: runner.description } : {}), + status: "missing" as const, + npmSpec: runner.npmSpec, + }; + }); +} + +export function listQaRunnerCliContributions(): readonly QaRunnerCliContribution[] { + const contributions = new Map(); + + for (const runner of buildKnownQaRunnerCatalog()) { + contributions.set(runner.commandName, runner); + } for (const plugin of listDeclaredQaRunnerPlugins()) { const runtimeSurface = tryLoadActivatedBundledPluginPublicSurfaceModuleSync( @@ -83,21 +115,21 @@ export function listQaRunnerCliContributions(): readonly QaRunnerCliContribution const runtimeRegistrations = runtimeSurface ? listRuntimeRegistrations(plugin.id, runtimeSurface) : null; + const declaredCommandNames = new Set(plugin.qaRunners.map((runner) => runner.commandName)); for (const runner of plugin.qaRunners) { - const previousOwner = seenCommandNames.get(runner.commandName); - if (previousOwner) { + const previous = contributions.get(runner.commandName); + if (previous && previous.pluginId !== plugin.id) { throw new Error( - `QA runner command "${runner.commandName}" declared by both "${previousOwner}" and "${plugin.id}"`, + `QA runner command "${runner.commandName}" declared by both "${previous.pluginId}" and "${plugin.id}"`, ); } - seenCommandNames.set(runner.commandName, plugin.id); const registration = runtimeRegistrations?.find( (entry) => entry.commandName === runner.commandName, ); if (!runtimeSurface) { - contributions.push({ + contributions.set(runner.commandName, { pluginId: plugin.id, commandName: runner.commandName, ...(runner.description ? { description: runner.description } : {}), @@ -110,7 +142,7 @@ export function listQaRunnerCliContributions(): readonly QaRunnerCliContribution `QA runner plugin "${plugin.id}" declared "${runner.commandName}" in openclaw.plugin.json but did not export a matching CLI registration`, ); } - contributions.push({ + contributions.set(runner.commandName, { pluginId: plugin.id, commandName: runner.commandName, ...(runner.description ? { description: runner.description } : {}), @@ -118,7 +150,15 @@ export function listQaRunnerCliContributions(): readonly QaRunnerCliContribution registration, }); } + + for (const registration of runtimeRegistrations ?? []) { + if (!declaredCommandNames.has(registration.commandName)) { + throw new Error( + `QA runner plugin "${plugin.id}" exported "${registration.commandName}" from runtime-api.js but did not declare it in openclaw.plugin.json`, + ); + } + } } - return contributions; + return [...contributions.values()]; } diff --git a/src/plugins/qa-runner-catalog.ts b/src/plugins/qa-runner-catalog.ts new file mode 100644 index 00000000000..dd0bba3a460 --- /dev/null +++ b/src/plugins/qa-runner-catalog.ts @@ -0,0 +1,74 @@ +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { listBundledPluginMetadata } from "./bundled-plugin-metadata.js"; + +export type QaRunnerCatalogEntry = { + pluginId: string; + commandName: string; + description?: string; + npmSpec: string; +}; + +const QA_RUNNER_CATALOG_JSON_PATH = fileURLToPath( + new URL("../../scripts/lib/qa-runner-catalog.json", import.meta.url), +); + +export function listBundledQaRunnerCatalog(): readonly QaRunnerCatalogEntry[] { + if (!fs.existsSync(QA_RUNNER_CATALOG_JSON_PATH)) { + return []; + } + return JSON.parse(fs.readFileSync(QA_RUNNER_CATALOG_JSON_PATH, "utf8")) as QaRunnerCatalogEntry[]; +} + +export function collectBundledQaRunnerCatalog(params?: { + rootDir?: string; +}): readonly QaRunnerCatalogEntry[] { + const catalog: QaRunnerCatalogEntry[] = []; + const seenCommandNames = new Map(); + + for (const entry of listBundledPluginMetadata({ + rootDir: params?.rootDir, + includeChannelConfigs: false, + })) { + const qaRunners = entry.manifest.qaRunners ?? []; + const npmSpec = entry.packageManifest?.install?.npmSpec?.trim() || entry.packageName?.trim(); + if (!npmSpec) { + continue; + } + for (const runner of qaRunners) { + const previousOwner = seenCommandNames.get(runner.commandName); + if (previousOwner) { + throw new Error( + `QA runner command "${runner.commandName}" declared by both "${previousOwner}" and "${entry.manifest.id}"`, + ); + } + seenCommandNames.set(runner.commandName, entry.manifest.id); + catalog.push({ + pluginId: entry.manifest.id, + commandName: runner.commandName, + ...(runner.description ? { description: runner.description } : {}), + npmSpec, + }); + } + } + + return catalog.toSorted((left, right) => left.commandName.localeCompare(right.commandName)); +} + +export async function writeBundledQaRunnerCatalog(params: { + repoRoot: string; + check: boolean; +}): Promise<{ changed: boolean; jsonPath: string }> { + const jsonPath = path.join(params.repoRoot, "scripts", "lib", "qa-runner-catalog.json"); + const expectedJson = `${JSON.stringify(collectBundledQaRunnerCatalog({ rootDir: params.repoRoot }), null, 2)}\n`; + const currentJson = fs.existsSync(jsonPath) ? fs.readFileSync(jsonPath, "utf8") : ""; + const changed = currentJson !== expectedJson; + + if (!params.check && changed) { + fs.mkdirSync(path.dirname(jsonPath), { recursive: true }); + fs.writeFileSync(jsonPath, expectedJson, "utf8"); + } + + return { changed, jsonPath }; +}