fix(matrix): address QA triage findings

This commit is contained in:
Gustavo Madeira Santana
2026-04-25 15:52:37 -04:00
parent aec062767f
commit 2c8a3df03d
7 changed files with 121 additions and 58 deletions

View File

@@ -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 () => { it("describes reattached generated images in the roundtrip flow", async () => {
const server = await startQaMockOpenAiServer({ const server = await startQaMockOpenAiServer({
host: "127.0.0.1", host: "127.0.0.1",

View File

@@ -339,21 +339,34 @@ function extractAllRequestTexts(input: ResponsesInputItem[], body: Record<string
} }
function countImageInputs(value: unknown): number { function countImageInputs(value: unknown): number {
if (Array.isArray(value)) { const seen = new WeakSet<object>();
return value.reduce((sum, entry) => sum + countImageInputs(entry), 0); 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<string, unknown>;
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 count;
return 0;
}
const record = value as Record<string, unknown>;
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;
} }
function parseToolOutputJson(toolOutput: string): Record<string, unknown> | null { function parseToolOutputJson(toolOutput: string): Record<string, unknown> | null {

View File

@@ -24,6 +24,9 @@ describe("Matrix QA CLI runtime", () => {
expect(formatMatrixQaCliCommand(["matrix", "account", "add", "--access-token=token-123"])).toBe( expect(formatMatrixQaCliCommand(["matrix", "account", "add", "--access-token=token-123"])).toBe(
"openclaw matrix account add --access-token=[REDACTED]", "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", () => { it("redacts Matrix token output before diagnostics and artifacts", () => {

View File

@@ -2,11 +2,11 @@ import { spawn as startOpenClawCliProcess } from "node:child_process";
import { randomUUID } from "node:crypto"; import { randomUUID } from "node:crypto";
import { existsSync } from "node:fs"; import { existsSync } from "node:fs";
import { chmod, mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; import { chmod, mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import path from "node:path"; import path from "node:path";
import { setTimeout as sleep } from "node:timers/promises"; import { setTimeout as sleep } from "node:timers/promises";
import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime";
import { redactSensitiveText } from "openclaw/plugin-sdk/logging-core"; import { redactSensitiveText } from "openclaw/plugin-sdk/logging-core";
import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path";
export type MatrixQaCliRunResult = { export type MatrixQaCliRunResult = {
args: string[]; args: string[];
@@ -30,6 +30,10 @@ export type MatrixQaCliSession = {
const MATRIX_QA_CLI_SECRET_ARG_FLAGS = new Set(["--access-token", "--password", "--recovery-key"]); 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[] { function redactMatrixQaCliArgs(args: string[]): string[] {
return args.map((arg, index) => { return args.map((arg, index) => {
const [flag] = arg.split("=", 1); const [flag] = arg.split("=", 1);
@@ -40,6 +44,9 @@ function redactMatrixQaCliArgs(args: string[]): string[] {
if (previous && MATRIX_QA_CLI_SECRET_ARG_FLAGS.has(previous)) { if (previous && MATRIX_QA_CLI_SECRET_ARG_FLAGS.has(previous)) {
return "[REDACTED]"; return "[REDACTED]";
} }
if (isMatrixQaCliSecretPositionalArg(args, index)) {
return "[REDACTED]";
}
return arg; return arg;
}); });
} }
@@ -249,7 +256,9 @@ export async function createMatrixQaOpenClawCliRuntime(params: {
runtimeEnv: NodeJS.ProcessEnv; runtimeEnv: NodeJS.ProcessEnv;
userId: string; 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( const artifactDir = path.join(
params.outputDir, params.outputDir,
params.artifactLabel.replace(/[^A-Za-z0-9_-]/g, "-"), params.artifactLabel.replace(/[^A-Za-z0-9_-]/g, "-"),

View File

@@ -8,6 +8,7 @@ import {
createMatrixQaE2eeScenarioClient, createMatrixQaE2eeScenarioClient,
type MatrixQaE2eeScenarioClient, type MatrixQaE2eeScenarioClient,
} from "../../substrate/e2ee-client.js"; } from "../../substrate/e2ee-client.js";
import { requestMatrixJson } from "../../substrate/request.js";
import { import {
buildMatrixQaE2eeScenarioRoomKey, buildMatrixQaE2eeScenarioRoomKey,
type MatrixQaE2eeScenarioId, type MatrixQaE2eeScenarioId,
@@ -634,19 +635,14 @@ async function deleteMatrixQaServerRoomKeyBackup(params: {
baseUrl: string; baseUrl: string;
version: string; version: string;
}) { }) {
const endpoint = new URL( const response = await requestMatrixJson<Record<string, never>>({
`/_matrix/client/v3/room_keys/version/${encodeURIComponent(params.version)}`, accessToken: params.accessToken,
params.baseUrl, baseUrl: params.baseUrl,
); endpoint: `/_matrix/client/v3/room_keys/version/${encodeURIComponent(params.version)}`,
const response = await fetch(endpoint, { fetchImpl: fetch,
method: "DELETE", method: "DELETE",
headers: { okStatuses: [200, 404],
Authorization: `Bearer ${params.accessToken}`,
},
}); });
if (!response.ok && response.status !== 404) {
throw new Error(`Matrix server backup delete failed with HTTP ${response.status}`);
}
return response.status; return response.status;
} }
@@ -654,7 +650,6 @@ async function runMatrixQaExternalKeyRestore(params: {
accountId: string; accountId: string;
context: MatrixQaScenarioContext; context: MatrixQaScenarioContext;
deviceName: string; deviceName: string;
encodedRecoveryKey: string;
label: string; label: string;
password: string; password: string;
userId: string; userId: string;
@@ -688,7 +683,6 @@ export async function runMatrixQaE2eeStateLossExternalRecoveryKeyScenario(
accountId: "external-key", accountId: "external-key",
context, context,
deviceName: "OpenClaw Matrix QA External Key Restore", deviceName: "OpenClaw Matrix QA External Key Restore",
encodedRecoveryKey: setup.encodedRecoveryKey,
label: "state-loss-external-recovery-key", label: "state-loss-external-recovery-key",
password: driverPassword, password: driverPassword,
userId: context.driverUserId, userId: context.driverUserId,
@@ -776,7 +770,9 @@ export async function runMatrixQaE2eeStateLossExternalRecoveryKeyScenario(
} }
return { return {
artifacts: { artifacts: {
completedVerificationId: selfVerification?.completedOwner.id ?? null, ...(selfVerification
? { completedVerificationId: selfVerification.completedOwner.id }
: {}),
recoveryDeviceId: device.deviceId, recoveryDeviceId: device.deviceId,
recoveryKeyId: setup.recoveryKeyId, recoveryKeyId: setup.recoveryKeyId,
restoreImported: restored.payload.imported, restoreImported: restored.payload.imported,
@@ -826,7 +822,6 @@ export async function runMatrixQaE2eeStateLossStoredRecoveryKeyScenario(
accountId: "stored-key", accountId: "stored-key",
context, context,
deviceName: "OpenClaw Matrix QA Stored Key Restore", deviceName: "OpenClaw Matrix QA Stored Key Restore",
encodedRecoveryKey: setup.encodedRecoveryKey,
label: "state-loss-stored-recovery-key", label: "state-loss-stored-recovery-key",
password: driverPassword, password: driverPassword,
userId: context.driverUserId, userId: context.driverUserId,
@@ -908,7 +903,6 @@ export async function runMatrixQaE2eeStateLossNoRecoveryKeyScenario(
accountId: "no-key", accountId: "no-key",
context, context,
deviceName: "OpenClaw Matrix QA No Key Restore", deviceName: "OpenClaw Matrix QA No Key Restore",
encodedRecoveryKey: setup.encodedRecoveryKey,
label: "state-loss-no-recovery-key", label: "state-loss-no-recovery-key",
password: driverPassword, password: driverPassword,
userId: context.driverUserId, userId: context.driverUserId,
@@ -968,7 +962,6 @@ export async function runMatrixQaE2eeStaleRecoveryKeyAfterBackupResetScenario(
accountId: "stale-key", accountId: "stale-key",
context, context,
deviceName: "OpenClaw Matrix QA Stale Key Restore", deviceName: "OpenClaw Matrix QA Stale Key Restore",
encodedRecoveryKey: setup.encodedRecoveryKey,
label: "stale-recovery-key-after-backup-reset", label: "stale-recovery-key-after-backup-reset",
password: driverPassword, password: driverPassword,
userId: context.driverUserId, userId: context.driverUserId,
@@ -1076,6 +1069,7 @@ async function waitForMatrixQaNonEmptyCliBackupRestore(params: {
const startedAt = Date.now(); const startedAt = Date.now();
let last: Awaited<ReturnType<typeof runMatrixQaCliJson<MatrixQaCliBackupStatus>>> | null = null; let last: Awaited<ReturnType<typeof runMatrixQaCliJson<MatrixQaCliBackupStatus>>> | null = null;
while (Date.now() - startedAt < params.timeoutMs) { while (Date.now() - startedAt < params.timeoutMs) {
const remainingMs = params.timeoutMs - (Date.now() - startedAt);
const restored = await runMatrixQaCliJson<MatrixQaCliBackupStatus>({ const restored = await runMatrixQaCliJson<MatrixQaCliBackupStatus>({
args: [ args: [
"matrix", "matrix",
@@ -1090,7 +1084,7 @@ async function waitForMatrixQaNonEmptyCliBackupRestore(params: {
], ],
label: params.label, label: params.label,
runtime: params.cli, runtime: params.cli,
timeoutMs: params.timeoutMs, timeoutMs: Math.max(1, remainingMs),
}); });
last = restored; last = restored;
assertMatrixQaCliBackupRestoreSucceeded(restored.payload, params.label); assertMatrixQaCliBackupRestoreSucceeded(restored.payload, params.label);
@@ -1114,7 +1108,6 @@ export async function runMatrixQaE2eeServerBackupDeletedLocalReuploadRestoresSce
accountId: "backup-reupload", accountId: "backup-reupload",
context, context,
deviceName: "OpenClaw Matrix QA Backup Reupload Restore", deviceName: "OpenClaw Matrix QA Backup Reupload Restore",
encodedRecoveryKey: setup.encodedRecoveryKey,
label: "server-backup-deleted-local-reupload-restores", label: "server-backup-deleted-local-reupload-restores",
password: driverPassword, password: driverPassword,
userId: context.driverUserId, userId: context.driverUserId,
@@ -1191,7 +1184,6 @@ export async function runMatrixQaE2eeCorruptCryptoIdbSnapshotScenario(
accountId: "corrupt-idb", accountId: "corrupt-idb",
context, context,
deviceName: "OpenClaw Matrix QA Corrupt IDB Restore", deviceName: "OpenClaw Matrix QA Corrupt IDB Restore",
encodedRecoveryKey: setup.encodedRecoveryKey,
label: "corrupt-crypto-idb-snapshot", label: "corrupt-crypto-idb-snapshot",
password: driverPassword, password: driverPassword,
userId: context.driverUserId, userId: context.driverUserId,
@@ -1268,7 +1260,6 @@ export async function runMatrixQaE2eeServerDeviceDeletedLocalStateIntactScenario
accountId: "deleted-device", accountId: "deleted-device",
context, context,
deviceName: "OpenClaw Matrix QA Deleted Device", deviceName: "OpenClaw Matrix QA Deleted Device",
encodedRecoveryKey: setup.encodedRecoveryKey,
label: "server-device-deleted-local-state-intact", label: "server-device-deleted-local-state-intact",
password: driverPassword, password: driverPassword,
userId: context.driverUserId, userId: context.driverUserId,
@@ -1316,7 +1307,6 @@ export async function runMatrixQaE2eeServerDeviceDeletedLocalStateIntactScenario
return { return {
artifacts: { artifacts: {
deletedDeviceId: device.deviceId, deletedDeviceId: device.deviceId,
ownerDeviceListContainsDeletedDevice,
serverDeviceKnown: status.payload.serverDeviceKnown ?? null, serverDeviceKnown: status.payload.serverDeviceKnown ?? null,
statusError: status.payload.error, statusError: status.payload.error,
statusExitCode: status.result.exitCode, statusExitCode: status.result.exitCode,
@@ -1438,21 +1428,23 @@ export async function runMatrixQaE2eeWrongAccountRecoveryKeyScenario(
client: observer, client: observer,
label: "observer", label: "observer",
}); });
const device = await loginMatrixQaRecoveryDevice({ let device: Awaited<ReturnType<typeof loginMatrixQaRecoveryDevice>> | undefined;
context, let cli: Awaited<ReturnType<typeof createMatrixQaRecoveryCliRuntime>> | undefined;
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,
});
try { 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<MatrixQaCliBackupStatus>({ const restored = await runMatrixQaCliJson<MatrixQaCliBackupStatus>({
allowNonZero: true, allowNonZero: true,
args: [ args: [
@@ -1484,8 +1476,10 @@ export async function runMatrixQaE2eeWrongAccountRecoveryKeyScenario(
].join("\n"), ].join("\n"),
}; };
} finally { } finally {
await cli.dispose().catch(() => undefined); await cli?.dispose().catch(() => undefined);
await observer.deleteOwnDevices([device.deviceId]).catch(() => undefined); if (device) {
await observer.deleteOwnDevices([device.deviceId]).catch(() => undefined);
}
} }
} finally { } finally {
await observer.stop().catch(() => undefined); await observer.stop().catch(() => undefined);
@@ -1516,7 +1510,6 @@ export async function runMatrixQaE2eeHistoryExistsBackupEmptyScenario(
accountId: "empty-backup", accountId: "empty-backup",
context, context,
deviceName: "OpenClaw Matrix QA Empty Backup", deviceName: "OpenClaw Matrix QA Empty Backup",
encodedRecoveryKey: freshEncodedKey,
label: "history-exists-backup-empty", label: "history-exists-backup-empty",
password: driverPassword, password: driverPassword,
userId: context.driverUserId, userId: context.driverUserId,

View File

@@ -198,14 +198,15 @@ describe("matrix live qa scenarios", () => {
const destructiveIndex = scenarioIds.indexOf(destructiveScenarioId); const destructiveIndex = scenarioIds.indexOf(destructiveScenarioId);
expect(scenarioIds.at(-1)).toBe(destructiveScenarioId); expect(scenarioIds.at(-1)).toBe(destructiveScenarioId);
for (const scenarioId of [ const protectedScenarioIds = [
"matrix-e2ee-state-loss-external-recovery-key", "matrix-e2ee-state-loss-external-recovery-key",
"matrix-e2ee-state-loss-stored-recovery-key", "matrix-e2ee-state-loss-stored-recovery-key",
"matrix-e2ee-device-sas-verification", "matrix-e2ee-device-sas-verification",
"matrix-e2ee-qr-verification", "matrix-e2ee-qr-verification",
"matrix-e2ee-dm-sas-verification", "matrix-e2ee-dm-sas-verification",
"matrix-e2ee-media-image", "matrix-e2ee-media-image",
]) { ] satisfies (typeof scenarioIds)[number][];
for (const scenarioId of protectedScenarioIds) {
expect(destructiveIndex).toBeGreaterThan(scenarioIds.indexOf(scenarioId)); expect(destructiveIndex).toBeGreaterThan(scenarioIds.indexOf(scenarioId));
} }
}); });

View File

@@ -11,7 +11,7 @@ export async function requestMatrixJson<T>(params: {
body?: unknown; body?: unknown;
endpoint: string; endpoint: string;
fetchImpl: MatrixQaFetchLike; fetchImpl: MatrixQaFetchLike;
method: "GET" | "POST" | "PUT"; method: "DELETE" | "GET" | "POST" | "PUT";
okStatuses?: number[]; okStatuses?: number[];
query?: Record<string, string | number | undefined>; query?: Record<string, string | number | undefined>;
timeoutMs?: number; timeoutMs?: number;