fix(qa): Matrix replies disappear after final replacement (#117390)

* fix(qa): preserve Matrix relations through replacements

* fix: honor Matrix replacement relation semantics

* fix(qa): reconcile late Matrix replacement targets

* test(qa): exercise Matrix recorder through public behavior
This commit is contained in:
Dallin Romney
2026-08-01 21:27:15 +08:00
committed by GitHub
parent 355295bfef
commit 39cc0a3199
12 changed files with 234 additions and 96 deletions
@@ -286,10 +286,9 @@ export async function createMatrixQaTransportAdapter(
if (!logicalConversation) {
continue;
}
const replacedMessageId =
event.relatesTo?.relType === "m.replace" && event.relatesTo.eventId
? busMessageIds.get(event.relatesTo.eventId)
: undefined;
const replacedMessageId = event.replacesEventId
? busMessageIds.get(event.replacesEventId)
: undefined;
if (replacedMessageId) {
const outbound = await context.messages.editMessage({
accountId,
@@ -102,8 +102,7 @@ async function runMatrixStreamingPreviewScenario(
event.roomId === context.roomId &&
event.sender === context.sutUserId &&
isMatrixQaMessageLikeKind(event.kind) &&
event.relatesTo?.relType === "m.replace" &&
event.relatesTo.eventId === preview.event.eventId &&
event.replacesEventId === preview.event.eventId &&
event.body === params.finalText,
roomId: context.roomId,
since: preview.since,
@@ -133,8 +132,7 @@ async function runMatrixStreamingPreviewScenario(
`preview event: ${preview.event.eventId}`,
`preview kind: ${preview.event.kind}`,
`preview body: ${preview.event.body ?? "<none>"}`,
`final reply relation: ${finalized.event.relatesTo?.relType ?? "<none>"}`,
`final reply target: ${finalized.event.relatesTo?.eventId ?? "<none>"}`,
`final replacement target: ${finalized.event.replacesEventId ?? "<none>"}`,
...buildMatrixReplyDetails("final reply", finalReply),
].join("\n"),
} satisfies MatrixQaScenarioExecution;
@@ -47,7 +47,7 @@ describe("Matrix tool-progress timeout diagnostics", () => {
events: [
buildBoundaryEvent({
eventId: "$replacement",
relatesTo: { eventId: "$preview", relType: "m.replace" },
replacesEventId: "$preview",
}),
],
previewEventId: "$preview",
@@ -20,7 +20,7 @@ export function findMatrixQaUnexpectedWorkingEvents(params: {
if (event.eventId === params.previewEventId || event.eventId === params.finalEventId) {
return false;
}
return event.relatesTo?.eventId !== params.previewEventId;
return event.replacesEventId !== params.previewEventId;
});
}
@@ -66,6 +66,7 @@ function describeMatrixQaToolProgressCandidate(event: MatrixQaObservedEvent) {
: "<none>";
return [
`${event.eventId} kind=${event.kind}`,
`replaces=${event.replacesEventId ?? "<none>"}`,
`relation=${relation}`,
`body=${JSON.stringify(truncateMatrixQaToolProgressBody(event.body))}`,
].join(" ");
@@ -93,7 +94,7 @@ export function buildMatrixQaToolProgressTimeoutMessage(params: {
}
return (
event.eventId === params.previewEventId ||
event.relatesTo?.eventId === params.previewEventId ||
event.replacesEventId === params.previewEventId ||
event.body !== undefined
);
})
@@ -152,7 +153,7 @@ export function buildMatrixQaToolProgressFinalTimeoutMessage(params: {
) {
return false;
}
return event.relatesTo?.eventId === params.previewEventId;
return event.replacesEventId === params.previewEventId;
})
.slice(-8);
const candidateDetails =
@@ -59,13 +59,12 @@ async function runMatrixToolProgressScenario(
params.progressPattern.test(body ?? "") ||
(params.allowGenericProgressLine === true && hasMatrixQaToolProgressPreviewLine(body));
const getPreviewRootEventId = (event: MatrixQaObservedEvent) =>
event.relatesTo?.relType === "m.replace" && event.relatesTo.eventId
? event.relatesTo.eventId
: event.eventId;
event.replacesEventId ?? event.eventId;
const isFinalReply = (event: MatrixQaObservedEvent) =>
event.roomId === context.roomId &&
event.sender === context.sutUserId &&
event.type === "m.room.message" &&
event.replacesEventId === undefined &&
event.relatesTo === undefined &&
isMatrixQaMessageLikeKind(event.kind) &&
doesMatrixQaReplyBodyMatchToken(event, params.finalText);
@@ -78,7 +77,8 @@ async function runMatrixToolProgressScenario(
event.roomId === context.roomId &&
event.sender === context.sutUserId &&
isExpectedProgressKind(event) &&
(matchesExpectedProgress(event.body) || event.relatesTo === undefined);
(matchesExpectedProgress(event.body) ||
(event.replacesEventId === undefined && event.relatesTo === undefined));
const isProgressProofEvent = (event: MatrixQaObservedEvent) =>
event.roomId === context.roomId &&
event.sender === context.sutUserId &&
@@ -88,15 +88,13 @@ async function runMatrixToolProgressScenario(
event.roomId === context.roomId &&
event.sender === context.sutUserId &&
event.kind === params.expectedPreviewKind &&
event.relatesTo?.relType === "m.replace" &&
event.relatesTo.eventId === previewRootEventId &&
event.replacesEventId === previewRootEventId &&
matchesExpectedProgress(event.body);
const isFinalReplacement = (event: MatrixQaObservedEvent, previewRootEventId: string) =>
event.roomId === context.roomId &&
event.sender === context.sutUserId &&
isMatrixQaMessageLikeKind(event.kind) &&
event.relatesTo?.relType === "m.replace" &&
event.relatesTo.eventId === previewRootEventId &&
event.replacesEventId === previewRootEventId &&
doesMatrixQaReplyBodyMatchToken(event, params.finalText);
const throwProgressTimeout = (err: unknown, previewEventId: string): never => {
throw new Error(
@@ -290,9 +288,10 @@ async function runMatrixToolProgressScenario(
event.sender === context.sutUserId &&
isMatrixQaMessageLikeKind(event.kind) &&
doesMatrixQaReplyBodyMatchToken(event, params.finalText) &&
((event.relatesTo?.relType === "m.replace" &&
event.relatesTo.eventId === previewRootEventId) ||
(params.allowTopLevelFinalWithProgress === true && event.relatesTo === undefined)),
(event.replacesEventId === previewRootEventId ||
(params.allowTopLevelFinalWithProgress === true &&
event.replacesEventId === undefined &&
event.relatesTo === undefined)),
roomId: context.roomId,
since: progress.since,
timeoutMs: context.timeoutMs,
@@ -1,6 +1,6 @@
import fs from "node:fs/promises";
import path from "node:path";
import type { MatrixQaObservedEvent } from "./events.js";
import { inheritMatrixQaReplacementRelation, type MatrixQaObservedEvent } from "./events.js";
export type MatrixQaE2eeActorId = "driver" | "observer" | `driver-${string}` | `cli-${string}`;
@@ -40,7 +40,7 @@ export async function runMatrixQaE2eeClientOperation<T>(params: {
}
}
export function shouldRecordMatrixQaObservedEventUpdate(params: {
function shouldRecordMatrixQaObservedEventUpdate(params: {
next: MatrixQaObservedEvent;
previous: MatrixQaObservedEvent | undefined;
}) {
@@ -53,11 +53,75 @@ export function shouldRecordMatrixQaObservedEventUpdate(params: {
(previous.body === undefined && next.body !== undefined) ||
(previous.formattedBody === undefined && next.formattedBody !== undefined) ||
(previous.msgtype === undefined && next.msgtype !== undefined) ||
(previous.relatesTo === undefined && next.relatesTo !== undefined) ||
(previous.mentions === undefined && next.mentions !== undefined) ||
(previous.attachment === undefined && next.attachment !== undefined)
);
}
export function createMatrixQaE2eeObservedEventRecorder(params: {
append: (event: MatrixQaObservedEvent) => void;
}) {
const eventsById = new Map<string, MatrixQaObservedEvent>();
const replacementIdsByTargetId = new Map<string, Set<string>>();
const append = (event: MatrixQaObservedEvent) => {
eventsById.set(event.eventId, event);
params.append(event);
};
const rehydrateReplacements = (target: MatrixQaObservedEvent) => {
if (!target.relatesTo) {
return;
}
for (const replacementId of replacementIdsByTargetId.get(target.eventId) ?? []) {
const replacement = eventsById.get(replacementId);
if (!replacement || replacement.relatesTo) {
continue;
}
const rehydrated = inheritMatrixQaReplacementRelation({
event: replacement,
replacedEvent: target,
});
if (rehydrated !== replacement) {
// Waiters scan append-only history from a cursor, so relation enrichment
// must be observable as a new record rather than an in-place mutation.
append(rehydrated);
}
}
};
return {
record(normalized: MatrixQaObservedEvent | null) {
if (!normalized) {
return;
}
const observed = inheritMatrixQaReplacementRelation({
event: normalized,
replacedEvent: normalized.replacesEventId
? eventsById.get(normalized.replacesEventId)
: undefined,
});
if (
!shouldRecordMatrixQaObservedEventUpdate({
next: observed,
previous: eventsById.get(observed.eventId),
})
) {
return;
}
if (observed.replacesEventId) {
const replacementIds =
replacementIdsByTargetId.get(observed.replacesEventId) ?? new Set<string>();
replacementIds.add(observed.eventId);
replacementIdsByTargetId.set(observed.replacesEventId, replacementIds);
}
append(observed);
rehydrateReplacements(observed);
},
};
}
function buildMatrixQaE2eeStoragePaths(params: {
actorId: MatrixQaE2eeActorId;
outputDir: string;
@@ -5,18 +5,18 @@ import path from "node:path";
import { describe, expect, it, vi } from "vitest";
import {
MATRIX_QA_E2EE_SYNC_FILTER,
createMatrixQaE2eeObservedEventRecorder,
prepareMatrixQaE2eeStorage,
runMatrixQaE2eeClientOperation,
shouldRecordMatrixQaObservedEventUpdate,
} from "./e2ee-client-internals.js";
import { findMatrixQaObservedEventMatch } from "./events.js";
import { findMatrixQaObservedEventMatch, type MatrixQaObservedEvent } from "./events.js";
const testing = {
MATRIX_QA_E2EE_SYNC_FILTER,
createMatrixQaE2eeObservedEventRecorder,
findMatrixQaObservedEventMatch,
prepareMatrixQaE2eeStorage,
runMatrixQaE2eeClientOperation,
shouldRecordMatrixQaObservedEventUpdate,
};
describe("matrix qa e2ee client storage", () => {
@@ -105,30 +105,61 @@ describe("matrix qa e2ee client storage", () => {
sender: "@bot:matrix-qa.test",
type: "m.room.message",
};
const observed: MatrixQaObservedEvent[] = [];
const recorder = testing.createMatrixQaE2eeObservedEventRecorder({
append: (event) => observed.push(event),
});
const decrypted = {
...previous,
body: "MATRIX_QA_E2EE_CLI_GATEWAY_OK",
msgtype: "m.text",
};
expect(
testing.shouldRecordMatrixQaObservedEventUpdate({
previous,
next: {
...previous,
body: "MATRIX_QA_E2EE_CLI_GATEWAY_OK",
msgtype: "m.text",
},
}),
).toBe(true);
expect(
testing.shouldRecordMatrixQaObservedEventUpdate({
previous: {
...previous,
body: "MATRIX_QA_E2EE_CLI_GATEWAY_OK",
msgtype: "m.text",
},
next: {
...previous,
body: "MATRIX_QA_E2EE_CLI_GATEWAY_OK",
msgtype: "m.text",
},
}),
).toBe(false);
recorder.record(previous);
recorder.record(decrypted);
recorder.record(decrypted);
expect(observed).toEqual([previous, decrypted]);
});
it("rehydrates a replacement when its threaded target decrypts later", () => {
const observed: MatrixQaObservedEvent[] = [];
const recorder = testing.createMatrixQaE2eeObservedEventRecorder({
append: (event) => observed.push(event),
});
const replacement = {
eventId: "$final",
kind: "message" as const,
roomId: "!room:matrix-qa.test",
sender: "@bot:matrix-qa.test",
type: "m.room.message",
body: "final",
msgtype: "m.text",
replacesEventId: "$preview",
};
const relation = {
eventId: "$root",
inReplyToId: "$driver",
isFallingBack: true,
relType: "m.thread",
};
recorder.record(replacement);
recorder.record({
eventId: "$preview",
kind: "notice",
roomId: "!room:matrix-qa.test",
sender: "@bot:matrix-qa.test",
type: "m.room.message",
body: "preview",
msgtype: "m.notice",
relatesTo: relation,
});
expect(observed).toEqual([
replacement,
expect.objectContaining({ eventId: "$preview", relatesTo: relation }),
{ ...replacement, relatesTo: relation },
]);
});
});
@@ -25,9 +25,9 @@ import type {
import { buildMatrixQaMessageContent } from "./client-message-content.js";
import {
MATRIX_QA_E2EE_SYNC_FILTER,
createMatrixQaE2eeObservedEventRecorder,
prepareMatrixQaE2eeStorage,
runMatrixQaE2eeClientOperation,
shouldRecordMatrixQaObservedEventUpdate,
type MatrixQaE2eeActorId,
} from "./e2ee-client-internals.js";
import { findMatrixQaObservedEventMatch, normalizeMatrixQaObservedEvent } from "./events.js";
@@ -316,24 +316,18 @@ export async function createMatrixQaE2eeScenarioClient(
const client: MatrixClient = await createMatrixQaE2eeMatrixClient(params);
const localEvents: MatrixQaObservedEvent[] = [];
const verificationSummaries: MatrixVerificationSummary[] = [];
const observedEventsById = new Map<string, MatrixQaObservedEvent>();
let primeCursorIndex = 0;
const cursorIndexByRoom = new Map<string, number>();
const observedEventRecorder = createMatrixQaE2eeObservedEventRecorder({
append(event) {
localEvents.push(event);
params.observedEvents.push(event);
},
});
const recordEvent = (roomId: string, event: MatrixRawEvent) => {
const normalized = normalizeMatrixQaObservedEvent(roomId, event);
if (
!normalized ||
!shouldRecordMatrixQaObservedEventUpdate({
next: normalized,
previous: observedEventsById.get(normalized.eventId),
})
) {
return;
}
observedEventsById.set(normalized.eventId, normalized);
localEvents.push(normalized);
params.observedEvents.push(normalized);
observedEventRecorder.record(normalizeMatrixQaObservedEvent(roomId, event));
};
client.on("room.message", recordEvent);
const recordVerificationSummary = (summary: MatrixVerificationSummary) => {
@@ -85,6 +85,12 @@ describe("matrix observed event normalization", () => {
"m.new_content": {
body: "finalized",
msgtype: "m.text",
// Matrix ignores relations inside replacement content. The
// observer inherits the original event's relation separately.
"m.relates_to": {
rel_type: "m.thread",
event_id: "$wrong-root",
},
},
"m.relates_to": {
rel_type: "m.replace",
@@ -104,12 +110,7 @@ describe("matrix observed event normalization", () => {
formattedBody: undefined,
msgtype: "m.text",
membership: undefined,
relatesTo: {
eventId: "$draft",
inReplyToId: undefined,
isFallingBack: undefined,
relType: "m.replace",
},
replacesEventId: "$draft",
});
});
@@ -64,6 +64,7 @@ export type MatrixQaObservedEvent = {
eventId?: string;
key?: string;
};
replacesEventId?: string;
redactsEventId?: string;
attachment?: MatrixQaObservedEventAttachment;
approval?: MatrixQaObservedApproval;
@@ -93,6 +94,25 @@ function resolveMatrixQaMessageContent(
return content;
}
function normalizeMatrixQaRelation(value: unknown) {
if (typeof value !== "object" || value === null) {
return undefined;
}
const relation = value as Record<string, unknown>;
const inReplyToRaw = relation["m.in_reply_to"];
const inReplyTo =
typeof inReplyToRaw === "object" && inReplyToRaw !== null
? (inReplyToRaw as Record<string, unknown>)
: null;
return {
eventId: typeof relation.event_id === "string" ? relation.event_id : undefined,
inReplyToId: typeof inReplyTo?.event_id === "string" ? inReplyTo.event_id : undefined,
isFallingBack:
typeof relation.is_falling_back === "boolean" ? relation.is_falling_back : undefined,
relType: typeof relation.rel_type === "string" ? relation.rel_type : undefined,
};
}
function resolveMatrixQaObservedEventKind(params: { msgtype?: string; type: string }) {
if (params.type === "m.reaction") {
return "reaction" as const;
@@ -213,12 +233,15 @@ export function normalizeMatrixQaObservedEvent(
typeof relatesToRaw === "object" && relatesToRaw !== null
? (relatesToRaw as Record<string, unknown>)
: null;
const inReplyToRaw = relatesTo?.["m.in_reply_to"];
const inReplyTo =
typeof inReplyToRaw === "object" && inReplyToRaw !== null
? (inReplyToRaw as Record<string, unknown>)
: null;
const messageContent = resolveMatrixQaMessageContent(content, relatesTo);
const replacesEventId =
relatesTo?.rel_type === "m.replace" && typeof relatesTo.event_id === "string"
? relatesTo.event_id
: undefined;
// An edit's outer m.replace relation describes wire delivery, not the
// logical relation of the edited message. Matrix ignores relations inside
// m.new_content, so the observer must inherit the original event's relation.
const logicalRelation = replacesEventId ? undefined : normalizeMatrixQaRelation(relatesToRaw);
const normalizedMsgtype =
typeof messageContent.msgtype === "string" ? messageContent.msgtype : msgtype;
const normalizedFilename =
@@ -270,19 +293,7 @@ export function normalizeMatrixQaObservedEvent(
typeof messageContent.formatted_body === "string" ? messageContent.formatted_body : undefined,
msgtype: normalizedMsgtype,
membership: typeof content.membership === "string" ? content.membership : undefined,
...(relatesTo
? {
relatesTo: {
eventId: typeof relatesTo.event_id === "string" ? relatesTo.event_id : undefined,
inReplyToId: typeof inReplyTo?.event_id === "string" ? inReplyTo.event_id : undefined,
isFallingBack:
typeof relatesTo.is_falling_back === "boolean"
? relatesTo.is_falling_back
: undefined,
relType: typeof relatesTo.rel_type === "string" ? relatesTo.rel_type : undefined,
},
}
: {}),
...(logicalRelation ? { relatesTo: logicalRelation } : {}),
...(mentions
? {
mentions: {
@@ -300,11 +311,22 @@ export function normalizeMatrixQaObservedEvent(
}
: {}),
...(redactsEventId ? { redactsEventId } : {}),
...(replacesEventId ? { replacesEventId } : {}),
...(attachment ? { attachment } : {}),
...(approval ? { approval } : {}),
};
}
export function inheritMatrixQaReplacementRelation(params: {
event: MatrixQaObservedEvent;
replacedEvent?: MatrixQaObservedEvent;
}) {
if (!params.event.replacesEventId || params.event.relatesTo || !params.replacedEvent?.relatesTo) {
return params.event;
}
return { ...params.event, relatesTo: params.replacedEvent.relatesTo };
}
export function findMatrixQaObservedEventMatch(params: {
cursorIndex: number;
events: MatrixQaObservedEvent[];
@@ -196,7 +196,16 @@ describe("matrix sync helpers", () => {
event_id: "$preview",
sender: "@sut:matrix-qa.test",
type: "m.room.message",
content: { body: "preview", msgtype: "m.notice" },
content: {
body: "preview",
msgtype: "m.notice",
"m.relates_to": {
rel_type: "m.thread",
event_id: "$root",
is_falling_back: true,
"m.in_reply_to": { event_id: "$driver" },
},
},
},
{
event_id: "$final",
@@ -205,10 +214,10 @@ describe("matrix sync helpers", () => {
content: {
body: "final",
msgtype: "m.text",
"m.new_content": { body: "final", msgtype: "m.text" },
"m.relates_to": {
rel_type: "m.replace",
event_id: "$preview",
"m.new_content": { body: "final", msgtype: "m.text" },
},
},
},
@@ -242,7 +251,17 @@ describe("matrix sync helpers", () => {
});
expect(preview.event.eventId).toBe("$preview");
expect(finalized.event.eventId).toBe("$final");
expect(finalized.event).toMatchObject({
body: "final",
eventId: "$final",
replacesEventId: "$preview",
relatesTo: {
eventId: "$root",
inReplyToId: "$driver",
isFallingBack: true,
relType: "m.thread",
},
});
expect(calls).toBe(1);
});
@@ -1,6 +1,7 @@
// Qa Lab Matrix module implements sync behavior.
import {
findMatrixQaObservedEventMatch,
inheritMatrixQaReplacementRelation,
normalizeMatrixQaObservedEvent,
type MatrixQaObservedEvent,
type MatrixQaRoomEvent,
@@ -57,6 +58,7 @@ export type MatrixQaRoomObserver = {
type MatrixQaRoomObserverState = {
cursorIndexes: Map<string, number>;
eventsById: Map<string, MatrixQaObservedEvent>;
events: MatrixQaObservedEvent[];
pollPromise?: Promise<void>;
since?: string;
@@ -108,8 +110,15 @@ async function pollMatrixQaRoomObserver(
if (!normalized) {
continue;
}
params.observedEvents.push(normalized);
params.roomObserver.events.push(normalized);
const observed = inheritMatrixQaReplacementRelation({
event: normalized,
replacedEvent: normalized.replacesEventId
? params.roomObserver.eventsById.get(normalized.replacesEventId)
: undefined,
});
params.roomObserver.eventsById.set(observed.eventId, observed);
params.observedEvents.push(observed);
params.roomObserver.events.push(observed);
}
}
})();
@@ -129,6 +138,7 @@ export function createMatrixQaRoomObserver(
): MatrixQaRoomObserver {
const roomObserver: MatrixQaRoomObserverState = {
cursorIndexes: new Map(),
eventsById: new Map(),
events: [],
since: params.since,
};