From 2c8a3df03d1f1c4802a2e3507bfe2212ac9da2ff Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Sat, 25 Apr 2026 15:52:37 -0400 Subject: [PATCH] fix(matrix): address QA triage findings --- .../src/providers/mock-openai/server.test.ts | 44 ++++++++++++ .../src/providers/mock-openai/server.ts | 41 +++++++---- .../contract/scenario-runtime-cli.test.ts | 3 + .../runners/contract/scenario-runtime-cli.ts | 13 +++- .../scenario-runtime-e2ee-destructive.ts | 71 +++++++++---------- .../src/runners/contract/scenarios.test.ts | 5 +- extensions/qa-matrix/src/substrate/request.ts | 2 +- 7 files changed, 121 insertions(+), 58 deletions(-) diff --git a/extensions/qa-lab/src/providers/mock-openai/server.test.ts b/extensions/qa-lab/src/providers/mock-openai/server.test.ts index 68417f8e4fc..e6f1d529c8c 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.test.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.test.ts @@ -1546,6 +1546,50 @@ describe("qa mock openai server", () => { }); }); + it("handles deeply nested image input shapes without recursive traversal failure", async () => { + const server = await startQaMockOpenAiServer({ + host: "127.0.0.1", + port: 0, + }); + cleanups.push(async () => { + await server.stop(); + }); + + let content: unknown = { + type: "input_image", + source: { + type: "base64", + mime_type: "image/png", + data: QA_IMAGE_PNG_BASE64, + }, + }; + for (let index = 0; index < 4_000; index += 1) { + content = [{ type: "input_text", text: "nested" }, content]; + } + + const response = await fetch(`${server.baseUrl}/v1/responses`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + stream: false, + model: "mock-openai/gpt-5.4", + input: [ + { + role: "user", + content, + }, + ], + }), + }); + expect(response.status).toBe(200); + + const debug = await fetch(`${server.baseUrl}/debug/last-request`); + expect(debug.status).toBe(200); + expect(await debug.json()).toMatchObject({ + imageInputCount: 1, + }); + }); + it("describes reattached generated images in the roundtrip flow", async () => { const server = await startQaMockOpenAiServer({ host: "127.0.0.1", diff --git a/extensions/qa-lab/src/providers/mock-openai/server.ts b/extensions/qa-lab/src/providers/mock-openai/server.ts index 3e61448445b..233f99dba2f 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.ts @@ -339,21 +339,34 @@ function extractAllRequestTexts(input: ResponsesInputItem[], body: Record sum + countImageInputs(entry), 0); + const seen = new WeakSet(); + const stack = [value]; + let count = 0; + let visited = 0; + while (stack.length > 0 && visited < 50_000) { + visited += 1; + const current = stack.pop(); + if (Array.isArray(current)) { + for (const entry of current) { + stack.push(entry); + } + continue; + } + if (!current || typeof current !== "object") { + continue; + } + if (seen.has(current)) { + continue; + } + seen.add(current); + const record = current as Record; + const type = typeof record.type === "string" ? record.type : ""; + if (type === "input_image" || type === "image" || type === "image_url" || type === "media") { + count += 1; + } + stack.push(record.content, record.image_url, record.source); } - if (!value || typeof value !== "object") { - return 0; - } - const record = value as Record; - const type = typeof record.type === "string" ? record.type : ""; - const imageLikeType = - type === "input_image" || type === "image" || type === "image_url" || type === "media"; - const nested = - countImageInputs(record.content) + - countImageInputs(record.image_url) + - countImageInputs(record.source); - return (imageLikeType ? 1 : 0) + nested; + return count; } function parseToolOutputJson(toolOutput: string): Record | null { diff --git a/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.test.ts b/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.test.ts index ed781b42cd2..14c301c507f 100644 --- a/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.test.ts +++ b/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.test.ts @@ -24,6 +24,9 @@ describe("Matrix QA CLI runtime", () => { expect(formatMatrixQaCliCommand(["matrix", "account", "add", "--access-token=token-123"])).toBe( "openclaw matrix account add --access-token=[REDACTED]", ); + expect( + formatMatrixQaCliCommand(["matrix", "verify", "device", "abcdef1234567890ghij", "--json"]), + ).toBe("openclaw matrix verify device [REDACTED] --json"); }); it("redacts Matrix token output before diagnostics and artifacts", () => { diff --git a/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.ts b/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.ts index 0a12122262b..deeec589718 100644 --- a/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.ts +++ b/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.ts @@ -2,11 +2,11 @@ import { spawn as startOpenClawCliProcess } from "node:child_process"; import { randomUUID } from "node:crypto"; import { existsSync } from "node:fs"; import { chmod, mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; -import { tmpdir } from "node:os"; import path from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; import { redactSensitiveText } from "openclaw/plugin-sdk/logging-core"; +import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path"; export type MatrixQaCliRunResult = { args: string[]; @@ -30,6 +30,10 @@ export type MatrixQaCliSession = { const MATRIX_QA_CLI_SECRET_ARG_FLAGS = new Set(["--access-token", "--password", "--recovery-key"]); +function isMatrixQaCliSecretPositionalArg(args: string[], index: number): boolean { + return args[0] === "matrix" && args[1] === "verify" && args[2] === "device" && index === 3; +} + function redactMatrixQaCliArgs(args: string[]): string[] { return args.map((arg, index) => { const [flag] = arg.split("=", 1); @@ -40,6 +44,9 @@ function redactMatrixQaCliArgs(args: string[]): string[] { if (previous && MATRIX_QA_CLI_SECRET_ARG_FLAGS.has(previous)) { return "[REDACTED]"; } + if (isMatrixQaCliSecretPositionalArg(args, index)) { + return "[REDACTED]"; + } return arg; }); } @@ -249,7 +256,9 @@ export async function createMatrixQaOpenClawCliRuntime(params: { runtimeEnv: NodeJS.ProcessEnv; userId: string; }) { - const rootDir = await mkdtemp(path.join(tmpdir(), "openclaw-matrix-cli-qa-")); + const rootDir = await mkdtemp( + path.join(resolvePreferredOpenClawTmpDir(), "openclaw-matrix-cli-qa-"), + ); const artifactDir = path.join( params.outputDir, params.artifactLabel.replace(/[^A-Za-z0-9_-]/g, "-"), diff --git a/extensions/qa-matrix/src/runners/contract/scenario-runtime-e2ee-destructive.ts b/extensions/qa-matrix/src/runners/contract/scenario-runtime-e2ee-destructive.ts index b322682c0d3..fce8cf56b5f 100644 --- a/extensions/qa-matrix/src/runners/contract/scenario-runtime-e2ee-destructive.ts +++ b/extensions/qa-matrix/src/runners/contract/scenario-runtime-e2ee-destructive.ts @@ -8,6 +8,7 @@ import { createMatrixQaE2eeScenarioClient, type MatrixQaE2eeScenarioClient, } from "../../substrate/e2ee-client.js"; +import { requestMatrixJson } from "../../substrate/request.js"; import { buildMatrixQaE2eeScenarioRoomKey, type MatrixQaE2eeScenarioId, @@ -634,19 +635,14 @@ async function deleteMatrixQaServerRoomKeyBackup(params: { baseUrl: string; version: string; }) { - const endpoint = new URL( - `/_matrix/client/v3/room_keys/version/${encodeURIComponent(params.version)}`, - params.baseUrl, - ); - const response = await fetch(endpoint, { + const response = await requestMatrixJson>({ + accessToken: params.accessToken, + baseUrl: params.baseUrl, + endpoint: `/_matrix/client/v3/room_keys/version/${encodeURIComponent(params.version)}`, + fetchImpl: fetch, method: "DELETE", - headers: { - Authorization: `Bearer ${params.accessToken}`, - }, + okStatuses: [200, 404], }); - if (!response.ok && response.status !== 404) { - throw new Error(`Matrix server backup delete failed with HTTP ${response.status}`); - } return response.status; } @@ -654,7 +650,6 @@ async function runMatrixQaExternalKeyRestore(params: { accountId: string; context: MatrixQaScenarioContext; deviceName: string; - encodedRecoveryKey: string; label: string; password: string; userId: string; @@ -688,7 +683,6 @@ export async function runMatrixQaE2eeStateLossExternalRecoveryKeyScenario( accountId: "external-key", context, deviceName: "OpenClaw Matrix QA External Key Restore", - encodedRecoveryKey: setup.encodedRecoveryKey, label: "state-loss-external-recovery-key", password: driverPassword, userId: context.driverUserId, @@ -776,7 +770,9 @@ export async function runMatrixQaE2eeStateLossExternalRecoveryKeyScenario( } return { artifacts: { - completedVerificationId: selfVerification?.completedOwner.id ?? null, + ...(selfVerification + ? { completedVerificationId: selfVerification.completedOwner.id } + : {}), recoveryDeviceId: device.deviceId, recoveryKeyId: setup.recoveryKeyId, restoreImported: restored.payload.imported, @@ -826,7 +822,6 @@ export async function runMatrixQaE2eeStateLossStoredRecoveryKeyScenario( accountId: "stored-key", context, deviceName: "OpenClaw Matrix QA Stored Key Restore", - encodedRecoveryKey: setup.encodedRecoveryKey, label: "state-loss-stored-recovery-key", password: driverPassword, userId: context.driverUserId, @@ -908,7 +903,6 @@ export async function runMatrixQaE2eeStateLossNoRecoveryKeyScenario( accountId: "no-key", context, deviceName: "OpenClaw Matrix QA No Key Restore", - encodedRecoveryKey: setup.encodedRecoveryKey, label: "state-loss-no-recovery-key", password: driverPassword, userId: context.driverUserId, @@ -968,7 +962,6 @@ export async function runMatrixQaE2eeStaleRecoveryKeyAfterBackupResetScenario( accountId: "stale-key", context, deviceName: "OpenClaw Matrix QA Stale Key Restore", - encodedRecoveryKey: setup.encodedRecoveryKey, label: "stale-recovery-key-after-backup-reset", password: driverPassword, userId: context.driverUserId, @@ -1076,6 +1069,7 @@ async function waitForMatrixQaNonEmptyCliBackupRestore(params: { const startedAt = Date.now(); let last: Awaited>> | null = null; while (Date.now() - startedAt < params.timeoutMs) { + const remainingMs = params.timeoutMs - (Date.now() - startedAt); const restored = await runMatrixQaCliJson({ args: [ "matrix", @@ -1090,7 +1084,7 @@ async function waitForMatrixQaNonEmptyCliBackupRestore(params: { ], label: params.label, runtime: params.cli, - timeoutMs: params.timeoutMs, + timeoutMs: Math.max(1, remainingMs), }); last = restored; assertMatrixQaCliBackupRestoreSucceeded(restored.payload, params.label); @@ -1114,7 +1108,6 @@ export async function runMatrixQaE2eeServerBackupDeletedLocalReuploadRestoresSce accountId: "backup-reupload", context, deviceName: "OpenClaw Matrix QA Backup Reupload Restore", - encodedRecoveryKey: setup.encodedRecoveryKey, label: "server-backup-deleted-local-reupload-restores", password: driverPassword, userId: context.driverUserId, @@ -1191,7 +1184,6 @@ export async function runMatrixQaE2eeCorruptCryptoIdbSnapshotScenario( accountId: "corrupt-idb", context, deviceName: "OpenClaw Matrix QA Corrupt IDB Restore", - encodedRecoveryKey: setup.encodedRecoveryKey, label: "corrupt-crypto-idb-snapshot", password: driverPassword, userId: context.driverUserId, @@ -1268,7 +1260,6 @@ export async function runMatrixQaE2eeServerDeviceDeletedLocalStateIntactScenario accountId: "deleted-device", context, deviceName: "OpenClaw Matrix QA Deleted Device", - encodedRecoveryKey: setup.encodedRecoveryKey, label: "server-device-deleted-local-state-intact", password: driverPassword, userId: context.driverUserId, @@ -1316,7 +1307,6 @@ export async function runMatrixQaE2eeServerDeviceDeletedLocalStateIntactScenario return { artifacts: { deletedDeviceId: device.deviceId, - ownerDeviceListContainsDeletedDevice, serverDeviceKnown: status.payload.serverDeviceKnown ?? null, statusError: status.payload.error, statusExitCode: status.result.exitCode, @@ -1438,21 +1428,23 @@ export async function runMatrixQaE2eeWrongAccountRecoveryKeyScenario( client: observer, label: "observer", }); - const device = await loginMatrixQaRecoveryDevice({ - context, - deviceName: "OpenClaw Matrix QA Wrong Account Key", - password: observerPassword, - userId: context.observerUserId, - }); - const cli = await createMatrixQaRecoveryCliRuntime({ - accountId: "wrong-account", - accessToken: device.accessToken, - context, - deviceId: device.deviceId, - label: "wrong-account-recovery-key", - userId: device.userId, - }); + let device: Awaited> | undefined; + let cli: Awaited> | undefined; try { + device = await loginMatrixQaRecoveryDevice({ + context, + deviceName: "OpenClaw Matrix QA Wrong Account Key", + password: observerPassword, + userId: context.observerUserId, + }); + cli = await createMatrixQaRecoveryCliRuntime({ + accountId: "wrong-account", + accessToken: device.accessToken, + context, + deviceId: device.deviceId, + label: "wrong-account-recovery-key", + userId: device.userId, + }); const restored = await runMatrixQaCliJson({ allowNonZero: true, args: [ @@ -1484,8 +1476,10 @@ export async function runMatrixQaE2eeWrongAccountRecoveryKeyScenario( ].join("\n"), }; } finally { - await cli.dispose().catch(() => undefined); - await observer.deleteOwnDevices([device.deviceId]).catch(() => undefined); + await cli?.dispose().catch(() => undefined); + if (device) { + await observer.deleteOwnDevices([device.deviceId]).catch(() => undefined); + } } } finally { await observer.stop().catch(() => undefined); @@ -1516,7 +1510,6 @@ export async function runMatrixQaE2eeHistoryExistsBackupEmptyScenario( accountId: "empty-backup", context, deviceName: "OpenClaw Matrix QA Empty Backup", - encodedRecoveryKey: freshEncodedKey, label: "history-exists-backup-empty", password: driverPassword, userId: context.driverUserId, diff --git a/extensions/qa-matrix/src/runners/contract/scenarios.test.ts b/extensions/qa-matrix/src/runners/contract/scenarios.test.ts index 42e9d9f7b22..24acf8e13d8 100644 --- a/extensions/qa-matrix/src/runners/contract/scenarios.test.ts +++ b/extensions/qa-matrix/src/runners/contract/scenarios.test.ts @@ -198,14 +198,15 @@ describe("matrix live qa scenarios", () => { const destructiveIndex = scenarioIds.indexOf(destructiveScenarioId); expect(scenarioIds.at(-1)).toBe(destructiveScenarioId); - for (const scenarioId of [ + const protectedScenarioIds = [ "matrix-e2ee-state-loss-external-recovery-key", "matrix-e2ee-state-loss-stored-recovery-key", "matrix-e2ee-device-sas-verification", "matrix-e2ee-qr-verification", "matrix-e2ee-dm-sas-verification", "matrix-e2ee-media-image", - ]) { + ] satisfies (typeof scenarioIds)[number][]; + for (const scenarioId of protectedScenarioIds) { expect(destructiveIndex).toBeGreaterThan(scenarioIds.indexOf(scenarioId)); } }); diff --git a/extensions/qa-matrix/src/substrate/request.ts b/extensions/qa-matrix/src/substrate/request.ts index 8c09505d066..f8990c385ee 100644 --- a/extensions/qa-matrix/src/substrate/request.ts +++ b/extensions/qa-matrix/src/substrate/request.ts @@ -11,7 +11,7 @@ export async function requestMatrixJson(params: { body?: unknown; endpoint: string; fetchImpl: MatrixQaFetchLike; - method: "GET" | "POST" | "PUT"; + method: "DELETE" | "GET" | "POST" | "PUT"; okStatuses?: number[]; query?: Record; timeoutMs?: number;