From cc1c61034fc0ec783d280d67f455b4d8e525318a Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Tue, 25 Aug 2026 18:02:17 -0700 Subject: [PATCH] fix(qa): publish readiness only from final artifacts (#124189) * fix(qa): publish readiness from final artifacts * fix(qa): isolate runtime parity readiness * test(qa): type runtime parity artifact mock --- .../src/suite-run-isolated.cleanup.test.ts | 133 +++++++++++++++++- extensions/qa-lab/src/suite-run-isolated.ts | 1 - extensions/qa-lab/src/suite-run-standard.ts | 6 +- ...uite-runtime-parity-runner.cleanup.test.ts | 59 +++++++- .../qa-lab/src/suite-runtime-parity-runner.ts | 63 +++++---- 5 files changed, 222 insertions(+), 40 deletions(-) diff --git a/extensions/qa-lab/src/suite-run-isolated.cleanup.test.ts b/extensions/qa-lab/src/suite-run-isolated.cleanup.test.ts index 15bda920c963..d675d3327a22 100644 --- a/extensions/qa-lab/src/suite-run-isolated.cleanup.test.ts +++ b/extensions/qa-lab/src/suite-run-isolated.cleanup.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { createQaBusState } from "./bus-state.js"; import type { QaLabServerHandle } from "./lab-server.types.js"; import type { QaTransportAdapterFactory } from "./qa-transport-registry.js"; +import type { writeQaSuiteArtifacts } from "./suite-artifacts.js"; import { runQaFlowSuiteIsolated } from "./suite-run-isolated.js"; import { runQaFlowSuiteStandard } from "./suite-run-standard.js"; import { makeQaSuiteTestScenario } from "./suite-test-helpers.js"; @@ -25,7 +26,7 @@ const mocks = vi.hoisted(() => ({ getProcessRssBytes: () => null, stop: vi.fn(async () => {}), })), - writeQaSuiteArtifacts: vi.fn(async () => ({ + writeQaSuiteArtifacts: vi.fn(async () => ({ evidence: undefined, evidencePath: "/qa-output/qa-evidence.json", report: "", @@ -43,6 +44,27 @@ vi.mock("openclaw/plugin-sdk/ssrf-runtime", () => ({ vi.mock("./gateway-child.js", () => ({ startQaGatewayChild: mocks.startQaGatewayChild, })); +vi.mock("./crabline-transport.js", () => ({ + createQaCrablineTransportAdapter: vi.fn(async () => ({ + id: "telegram", + label: "Crabline Telegram", + accountId: "sut", + requiredPluginIds: [], + supportedActions: [], + sendInbound: vi.fn(async () => {}), + createGatewayConfig: () => ({}), + waitReady: vi.fn(async () => {}), + buildAgentDelivery: ({ target }: { target: string }) => ({ + channel: "telegram", + to: target, + replyChannel: "telegram", + replyTo: target, + }), + handleAction: vi.fn(async () => {}), + createReportNotes: () => [], + cleanup: vi.fn(async () => {}), + })), +})); vi.mock("./providers/server-runtime.js", () => ({ startQaProviderServer: vi.fn(async () => undefined), })); @@ -217,6 +239,115 @@ describe("isolated QA suite transport cleanup", () => { stderrWrite.mockRestore(); }); + it("keeps Crabline workers concurrent while publishing readiness only from the final aggregate", async () => { + const lab = createCleanupTestLab(); + const selection = { + capabilityMatrixPath: "crabline-fake-provider-capabilities.json", + channel: "telegram", + channelDriver: "crabline", + smokeArtifactPath: "crabline-fake-provider-smoke.json", + } as const; + let activeWorkers = 0; + let maxActiveWorkers = 0; + let releaseWorkers!: () => void; + const bothWorkersStarted = new Promise((resolve) => { + releaseWorkers = resolve; + }); + let releaseFirstScenario!: () => void; + const firstScenarioStarted = new Promise((resolve) => { + releaseFirstScenario = resolve; + }); + let releaseScenarioExecutions!: () => void; + const bothScenarioExecutionsStarted = new Promise((resolve) => { + releaseScenarioExecutions = resolve; + }); + const context = createCleanupTestContext(); + context.channelDriver = "crabline"; + context.concurrency = 2; + context.selectedScenarios = [ + makeQaSuiteTestScenario("first-crabline-scenario"), + makeQaSuiteTestScenario("second-crabline-scenario"), + ]; + const runScenario = vi + .fn() + .mockImplementation(async (_env, scenario) => { + if (scenario.id === "first-crabline-scenario") { + releaseFirstScenario(); + await bothScenarioExecutionsStarted; + } else { + releaseScenarioExecutions(); + } + return { + name: scenario.title, + status: "pass", + steps: [], + }; + }); + const runChild = vi.fn().mockImplementation(async (params) => { + if (!params) { + throw new Error("expected nested standard run params"); + } + activeWorkers += 1; + maxActiveWorkers = Math.max(maxActiveWorkers, activeWorkers); + if (activeWorkers === 2) { + releaseWorkers(); + } + await bothWorkersStarted; + const scenarioId = params?.scenarioIds?.[0] ?? "missing-scenario"; + if (scenarioId === "second-crabline-scenario") { + await firstScenarioStarted; + } + const scenario = context.selectedScenarios.find((candidate) => candidate.id === scenarioId); + if (!scenario) { + throw new Error(`missing scenario ${scenarioId}`); + } + try { + return await runQaFlowSuiteStandard( + params, + { + ...context, + startedAt: new Date("2026-08-04T00:00:01.000Z"), + outputDir: params.outputDir ?? `/qa-child/${scenarioId}`, + selectedScenarios: [scenario], + concurrency: 1, + }, + runScenario, + ); + } finally { + activeWorkers -= 1; + } + }); + + const result = await runQaFlowSuiteIsolated( + { + channelDriverSelection: selection, + channelId: "telegram", + lab, + startLab: async () => createCleanupTestLab(), + }, + context, + runChild, + ); + + expect(maxActiveWorkers).toBe(2); + expect(result.scenarios).toEqual([ + expect.objectContaining({ name: "first-crabline-scenario", status: "pass" }), + expect.objectContaining({ name: "second-crabline-scenario", status: "pass" }), + ]); + expect(runScenario).toHaveBeenCalledTimes(2); + expect(mocks.writeQaSuiteArtifacts).toHaveBeenCalledTimes(5); + for (const [nonFinalArtifacts] of mocks.writeQaSuiteArtifacts.mock.calls.slice(0, -1)) { + expect(nonFinalArtifacts).toMatchObject({ channel: "telegram", channelDriver: "crabline" }); + expect(nonFinalArtifacts.channelDriverSelection).toBeUndefined(); + } + const finalArtifacts = mocks.writeQaSuiteArtifacts.mock.calls.at(-1)?.[0]; + expect(finalArtifacts).toMatchObject({ + channel: "telegram", + channelDriver: "crabline", + channelDriverSelection: selection, + }); + }); + it("prints one generic completion after a real nested standard run and parent cleanup", async () => { const parentLab = createCleanupTestLab(); const childLab = createCleanupTestLab(); diff --git a/extensions/qa-lab/src/suite-run-isolated.ts b/extensions/qa-lab/src/suite-run-isolated.ts index c70f2d89eaf5..d47fad05a6e5 100644 --- a/extensions/qa-lab/src/suite-run-isolated.ts +++ b/extensions/qa-lab/src/suite-run-isolated.ts @@ -114,7 +114,6 @@ export async function runQaFlowSuiteIsolated( concurrency, channel: params?.channelId ?? params?.channelDriverSelection?.channel ?? transport.id, channelDriver: transportFactoryResult.driver, - channelDriverSelection: params?.channelDriverSelection, isolatedWorkers: true, writeEvidenceFile: false, scenarioIds: diff --git a/extensions/qa-lab/src/suite-run-standard.ts b/extensions/qa-lab/src/suite-run-standard.ts index 4fa98f50bdeb..8d2606f60c23 100644 --- a/extensions/qa-lab/src/suite-run-standard.ts +++ b/extensions/qa-lab/src/suite-run-standard.ts @@ -400,7 +400,11 @@ export async function runQaFlowSuiteStandard( concurrency, channel: params?.channelId ?? params?.channelDriverSelection?.channel ?? transport.id, channelDriver: transportFactoryResult.driver, - channelDriverSelection: params?.channelDriverSelection, + // Nested workers retain the selection for transport setup, but the outer + // aggregate alone owns readiness publication under the shared output tree. + channelDriverSelection: isQaSuiteNestedRun(params) + ? undefined + : params?.channelDriverSelection, isolatedWorkers: false, writeEvidenceFile: params?.writeEvidenceFile, // Same "filtered → executed list, unfiltered → null" convention as diff --git a/extensions/qa-lab/src/suite-runtime-parity-runner.cleanup.test.ts b/extensions/qa-lab/src/suite-runtime-parity-runner.cleanup.test.ts index 46862ccf0b95..b2239839c9fc 100644 --- a/extensions/qa-lab/src/suite-runtime-parity-runner.cleanup.test.ts +++ b/extensions/qa-lab/src/suite-runtime-parity-runner.cleanup.test.ts @@ -48,13 +48,19 @@ const mocks = vi.hoisted(() => ({ getProcessRssBytes: () => null, stop: vi.fn(async () => {}), })), - writeQaSuiteArtifacts: vi.fn(async () => ({ - evidence: { kind: "test" }, - evidencePath: "/qa-output/qa-evidence.json", - report: "", - reportPath: "/qa-output/qa-suite-report.md", - summaryPath: "/qa-output/qa-suite-summary.json", - })), + writeQaSuiteArtifacts: vi.fn( + async (_params: { + channel?: string | null; + channelDriver?: string | null; + channelDriverSelection?: unknown; + }) => ({ + evidence: { kind: "test" }, + evidencePath: "/qa-output/qa-evidence.json", + report: "", + reportPath: "/qa-output/qa-suite-report.md", + summaryPath: "/qa-output/qa-suite-summary.json", + }), + ), })); vi.mock("openclaw/plugin-sdk/agent-harness", () => ({ @@ -63,6 +69,27 @@ vi.mock("openclaw/plugin-sdk/agent-harness", () => ({ vi.mock("openclaw/plugin-sdk/ssrf-runtime", () => ({ fetchWithSsrFGuard: mocks.fetchWithSsrFGuard, })); +vi.mock("./crabline-transport.js", () => ({ + createQaCrablineTransportAdapter: vi.fn(async () => ({ + id: "telegram", + label: "Crabline Telegram", + accountId: "sut", + requiredPluginIds: [], + supportedActions: [], + sendInbound: vi.fn(async () => {}), + createGatewayConfig: () => ({}), + waitReady: vi.fn(async () => {}), + buildAgentDelivery: ({ target }: { target: string }) => ({ + channel: "telegram", + to: target, + replyChannel: "telegram", + replyTo: target, + }), + handleAction: vi.fn(async () => {}), + createReportNotes: () => [], + cleanup: vi.fn(async () => {}), + })), +})); vi.mock("./gateway-child.js", () => ({ startQaGatewayChild: mocks.startQaGatewayChild, })); @@ -254,7 +281,14 @@ describe("runtime parity suite transport cleanup", () => { }); it("prints one generic completion after real nested standard cells and parent cleanup", async () => { + mocks.writeQaSuiteArtifacts.mockClear(); const scenario = makeQaSuiteTestScenario("runtime-cleanup"); + const selection = { + capabilityMatrixPath: "crabline-fake-provider-capabilities.json", + channel: "telegram", + channelDriver: "crabline", + smokeArtifactPath: "crabline-fake-provider-smoke.json", + } as const; const parentLab = createCleanupTestLab(); const openClawLab = createCleanupTestLab(); const codexLab = createCleanupTestLab(); @@ -300,6 +334,8 @@ describe("runtime parity suite transport cleanup", () => { startedAt: new Date("2026-08-04T00:00:00.000Z"), providerMode: "mock-openai", transportId: "qa-channel", + channelId: "telegram", + channelDriverSelection: selection, primaryModel: "mock-openai/test-model", alternateModel: "mock-openai/test-model-alt", fastMode: true, @@ -317,6 +353,15 @@ describe("runtime parity suite transport cleanup", () => { .filter((line) => line.startsWith("[qa-suite] run complete")); expect(completionLines).toEqual(["[qa-suite] run complete"]); expect(runScenario).toHaveBeenCalledTimes(2); + expect(mocks.writeQaSuiteArtifacts).toHaveBeenCalledTimes(3); + for (const [cellArtifacts] of mocks.writeQaSuiteArtifacts.mock.calls.slice(0, -1)) { + expect(cellArtifacts.channelDriverSelection).toBeUndefined(); + } + expect(mocks.writeQaSuiteArtifacts.mock.calls.at(-1)?.[0]).toMatchObject({ + channel: "telegram", + channelDriver: "crabline", + channelDriverSelection: selection, + }); expect(openClawLab.stop).toHaveBeenCalledOnce(); expect(codexLab.stop).toHaveBeenCalledOnce(); expect(parentLab.stop).toHaveBeenCalledOnce(); diff --git a/extensions/qa-lab/src/suite-runtime-parity-runner.ts b/extensions/qa-lab/src/suite-runtime-parity-runner.ts index 1878fe24ebe4..a9bc51b039a7 100644 --- a/extensions/qa-lab/src/suite-runtime-parity-runner.ts +++ b/extensions/qa-lab/src/suite-runtime-parity-runner.ts @@ -35,6 +35,7 @@ import type { } from "./suite-types.js"; import { createQaSuiteTransportAdapter, + markQaSuiteNestedRun, requireQaSuiteStartLab, runQaSuiteCleanupSteps, throwQaSuiteCleanupErrors, @@ -153,39 +154,41 @@ export async function runQaRuntimeParitySuite(params: { runtime, ); const cellStartedAt = Date.now(); - const cellResult = await params.runQaFlowSuite({ - adapterFactories: params.adapterFactories, - channelId: params.channelId, - adapterOptions: params.adapterOptions, - repoRoot: params.repoRoot, - outputDir: cellOutputDir, - providerMode: params.providerMode, - transportId: params.transportId, - channelDriver: params.channelDriver ?? undefined, - channelDriverSelection: params.channelDriverSelection, - primaryModel: remapModelRefForForcedRuntime({ - modelRef: params.primaryModel, + const cellResult = await params.runQaFlowSuite( + markQaSuiteNestedRun({ + adapterFactories: params.adapterFactories, + channelId: params.channelId, + adapterOptions: params.adapterOptions, + repoRoot: params.repoRoot, + outputDir: cellOutputDir, providerMode: params.providerMode, + transportId: params.transportId, + channelDriver: params.channelDriver ?? undefined, + channelDriverSelection: params.channelDriverSelection, + primaryModel: remapModelRefForForcedRuntime({ + modelRef: params.primaryModel, + providerMode: params.providerMode, + forcedRuntime: runtime, + }), + alternateModel: remapModelRefForForcedRuntime({ + modelRef: params.alternateModel, + providerMode: params.providerMode, + forcedRuntime: runtime, + }), + fastMode: params.fastMode, + thinkingDefault: params.thinkingDefault, + claudeCliAuthMode: params.claudeCliAuthMode, + scenarioIds: [scenario.id], + concurrency: 1, + enabledPluginIds: params.enabledPluginIds, + startLab, + controlUiEnabled: params.controlUiEnabled ?? scenarioRequiresControlUi(scenario), + mutateConfig: params.mutateConfig, forcedRuntime: runtime, + captureRuntimeParityCell: true, + writeEvidenceFile: params.writeEvidenceFile, }), - alternateModel: remapModelRefForForcedRuntime({ - modelRef: params.alternateModel, - providerMode: params.providerMode, - forcedRuntime: runtime, - }), - fastMode: params.fastMode, - thinkingDefault: params.thinkingDefault, - claudeCliAuthMode: params.claudeCliAuthMode, - scenarioIds: [scenario.id], - concurrency: 1, - enabledPluginIds: params.enabledPluginIds, - startLab, - controlUiEnabled: params.controlUiEnabled ?? scenarioRequiresControlUi(scenario), - mutateConfig: params.mutateConfig, - forcedRuntime: runtime, - captureRuntimeParityCell: true, - writeEvidenceFile: params.writeEvidenceFile, - }); + ); for (const startedScenarioId of cellResult.startedScenarioIds) { startedScenarioIds.add(startedScenarioId); }