borogove

created pr with borogove/call-fixes.1 on 2026-11-03T21:19:14Z · by 9e834e0d

added borogove/call-fixes.2 on 2026-11-09T14:58:54Z · by 9e834e0d


                      1:  dbd8243 ! 1:  a3fc6ef Never write ring before propose


                      2:  5ebad9d = 2:  5888d2b Presetve stanza ID for call updates


                      3:  9d8f8cc = 3:  fc1c6dd Get all versions of a call from sqlite

commented on 2026-11-09T15:01:23Z · by 3c31bc8c

lgtm

cmds

checkout latest patchset:

ssh pr.pico.sh pull borogove:call-fixes | git am -3

checkout specific patchset revision:

ssh pr.pico.sh pull borogove:call-fixes [rev] | git am -3

add changes to patch request:

git format-patch main --stdout | ssh pr.pico.sh borogove:call-fixes

add comment to patch request:

ssh pr.pico.sh comment borogove:call-fixes "lgtm!"

Patchset `borogove/call-fixes.2` on 2026-11-09T14:58:54Z · commit a3fc6ef

→ Never write ring before propose

Stephen Paul Weber 2026-11-03T20:10:13Z

Presetve stanza ID for call updates

Stephen Paul Weber 2026-11-03T20:39:43Z

Get all versions of a call from sqlite

Stephen Paul Weber 2026-11-03T21:13:30Z

Never write ring before propose

Stephen Paul Weber 2026-11-03T20:10:13Z

Ring is a sent event, in response to an inbound event, writing it first
would mess up which direction we think this call was in to begin with.

Semantic diff summary

0 added, 1 modified, 0 signature changed, 0 removed across 1 analyzed file (1 file skipped: unsupported file type)

test/persistence-tests.ts

function_declaration sharedPersistenceTests modified

+18 -13 borogove/Client.hx #

......@@ -541,12 +541,28 @@ class Client extends EventEmitter {
							}
						});
					};
					final maybeRing = () -> {
#if !NO_JINGLE
						final jmiP = stanza.getChild("propose", "urn:xmpp:jingle-message:0");
						if (jmiP == null || jmiP.attr.get("id") == null || from == null) return;
						final session = new IncomingProposedSession(this, from, jmiP.attr.get("id"));
						if (!chat.jingleSessions.exists(session.sid)) {
							chat.jingleSessions.set(session.sid, session);
							session.ring();
						}
#end
					};
					if (chatMessage.serverId == null) {
						updateChat(chatMessage);
						maybeRing();
					} else {
						maybeAutodownloadAttachments(chat, chatMessage).then(_ ->
							storeMessages([chatMessage]).then((stored) -> updateChat(stored[0]))
						);
							storeMessages([chatMessage])
						).then((stored) -> {
							updateChat(stored[0]);
							maybeRing();
							return true;
						});
					}
				}
			case ReactionUpdateStanza(update):

......@@ -620,17 +636,6 @@ class Client extends EventEmitter {
		if (stanza.attr.get("type") == "error") return;

#if !NO_JINGLE
		final jmiP = stanza.getChild("propose", "urn:xmpp:jingle-message:0");
		if (jmiP != null && jmiP.attr.get("id") != null) {
			final session = new IncomingProposedSession(this, from, jmiP.attr.get("id"));
			final chat = getDirectChat(from.asBare().asString());
			if (!chat.jingleSessions.exists(session.sid)) {
				chat.jingleSessions.set(session.sid, session);
				chatActivity(chat);
				session.ring();
			}
		}

		final jmiR = stanza.getChild("retract", "urn:xmpp:jingle-message:0");
		if (jmiR != null && jmiR.attr.get("id") != null) {
			final chat = getDirectChat(from.asBare().asString());

+83 -0 test/persistence-tests.ts #

......@@ -2290,6 +2290,89 @@ export function sharedPersistenceTests(test) {
			original.text,
		]);
	});

	test("ring-before-propose storage race keeps direction from the proposal", async ({
		borogove,
		persistence,
	}) => {
		const account = "race-alice@example.com";
		const contact = "race-hatter@example.com";
		const sid = "ring-before-propose";
		const client = new borogove.Client(account, persistence);
		const storeOrder: string[] = [];
		const originalStoreMessages = persistence.storeMessages.bind(persistence);
		let proposalStarted!: () => void;
		let releaseProposal!: () => void;
		const proposalStoreStarted = new Promise<void>((resolve) => {
			proposalStarted = resolve;
		});
		const proposalStoreCanFinish = new Promise<void>((resolve) => {
			releaseProposal = resolve;
		});
		let ringStoreStarted = false;
		let ringStoreFinished!: () => void;
		const ringStoreCanFinish = new Promise<void>((resolve) => {
			ringStoreFinished = resolve;
		});

		persistence.storeMessages = async (accountId, messages) => {
			const isProposal = messages.some(
				(message) =>
					message.type === borogove.MessageType.MessageCall &&
					message.callStatus() === "propose",
			);
			const isRinging = messages.some(
				(message) =>
					message.type === borogove.MessageType.MessageCall &&
					message.callStatus() === "ringing",
			);
			if (isProposal) {
				proposalStarted();
				await proposalStoreCanFinish;
			}
			if (isRinging) ringStoreStarted = true;
			const result = await originalStoreMessages(accountId, messages);
			for (const message of messages) {
				if (message.type === borogove.MessageType.MessageCall) {
					storeOrder.push(message.callStatus());
				}
			}
			if (isRinging) ringStoreFinished();
			return result;
		};

		try {
			(client as any).processLiveMessage(
				borogove.Stanza.parse(`
					<message xmlns="jabber:client" from="${contact}/phone" to="${account}" type="chat" id="proposal-message-id">
						<propose xmlns="urn:xmpp:jingle-message:0" id="${sid}">
							<description xmlns="urn:xmpp:jingle:apps:rtp:1" media="audio"/>
						</propose>
						<stanza-id xmlns="urn:xmpp:sid:0" by="${account}" id="race-propose-server-id"/>
					</message>
				`),
				null,
			);

			await proposalStoreStarted;
			// Let the real ring path run while the proposal write is still blocked.
			await new Promise((resolve) => setTimeout(resolve, 10));
			expect(ringStoreStarted).toBe(false);
			releaseProposal();
			// Wait for the ring write instead of assuming it finishes in a fixed time.
			await ringStoreCanFinish;

			expect(storeOrder).toEqual(["propose", "ringing"]);
			const stored = await persistence.getMessage(account, contact, null, sid);
			expect(stored).not.toBeNull();
			expect(stored?.direction).toBe(borogove.MessageDirection.MessageReceived);
			expect(stored?.senderId).toBe(contact);
		} finally {
			releaseProposal();
			persistence.storeMessages = originalStoreMessages;
		}
	});

}

type TestKeyPair = {

...	...		@@ -541,12 +541,28 @@ class Client extends EventEmitter {
541	541		}
542	542		});
543	543		};
	544	+	final maybeRing = () -> {
	545	+	#if !NO_JINGLE
	546	+	final jmiP = stanza.getChild("propose", "urn:xmpp:jingle-message:0");
	547	+	if (jmiP == null \|\| jmiP.attr.get("id") == null \|\| from == null) return;
	548	+	final session = new IncomingProposedSession(this, from, jmiP.attr.get("id"));
	549	+	if (!chat.jingleSessions.exists(session.sid)) {
	550	+	chat.jingleSessions.set(session.sid, session);
	551	+	session.ring();
	552	+	}
	553	+	#end
	554	+	};
544	555		if (chatMessage.serverId == null) {
545	556		updateChat(chatMessage);
	557	+	maybeRing();
546	558		} else {
547	559		maybeAutodownloadAttachments(chat, chatMessage).then(_ ->
548		-	storeMessages([chatMessage]).then((stored) -> updateChat(stored[0]))
549		-	);
	560	+	storeMessages([chatMessage])
	561	+	).then((stored) -> {
	562	+	updateChat(stored[0]);
	563	+	maybeRing();
	564	+	return true;
	565	+	});
550	566		}
551	567		}
552	568		case ReactionUpdateStanza(update):

...	...		@@ -620,17 +636,6 @@ class Client extends EventEmitter {
620	636		if (stanza.attr.get("type") == "error") return;
621	637
622	638		#if !NO_JINGLE
623		-	final jmiP = stanza.getChild("propose", "urn:xmpp:jingle-message:0");
624		-	if (jmiP != null && jmiP.attr.get("id") != null) {
625		-	final session = new IncomingProposedSession(this, from, jmiP.attr.get("id"));
626		-	final chat = getDirectChat(from.asBare().asString());
627		-	if (!chat.jingleSessions.exists(session.sid)) {
628		-	chat.jingleSessions.set(session.sid, session);
629		-	chatActivity(chat);
630		-	session.ring();
631		-	}
632		-	}
633		-
634	639		final jmiR = stanza.getChild("retract", "urn:xmpp:jingle-message:0");
635	640		if (jmiR != null && jmiR.attr.get("id") != null) {
636	641		final chat = getDirectChat(from.asBare().asString());

...	...		@@ -2290,6 +2290,89 @@ export function sharedPersistenceTests(test) {
2290	2290		original.text,
2291	2291		]);
2292	2292		});
	2293	+
	2294	+	test("ring-before-propose storage race keeps direction from the proposal", async ({
	2295	+	borogove,
	2296	+	persistence,
	2297	+	}) => {
	2298	+	const account = "race-alice@example.com";
	2299	+	const contact = "race-hatter@example.com";
	2300	+	const sid = "ring-before-propose";
	2301	+	const client = new borogove.Client(account, persistence);
	2302	+	const storeOrder: string[] = [];
	2303	+	const originalStoreMessages = persistence.storeMessages.bind(persistence);
	2304	+	let proposalStarted!: () => void;
	2305	+	let releaseProposal!: () => void;
	2306	+	const proposalStoreStarted = new Promise<void>((resolve) => {
	2307	+	proposalStarted = resolve;
	2308	+	});
	2309	+	const proposalStoreCanFinish = new Promise<void>((resolve) => {
	2310	+	releaseProposal = resolve;
	2311	+	});
	2312	+	let ringStoreStarted = false;
	2313	+	let ringStoreFinished!: () => void;
	2314	+	const ringStoreCanFinish = new Promise<void>((resolve) => {
	2315	+	ringStoreFinished = resolve;
	2316	+	});
	2317	+
	2318	+	persistence.storeMessages = async (accountId, messages) => {
	2319	+	const isProposal = messages.some(
	2320	+	(message) =>
	2321	+	message.type === borogove.MessageType.MessageCall &&
	2322	+	message.callStatus() === "propose",
	2323	+	);
	2324	+	const isRinging = messages.some(
	2325	+	(message) =>
	2326	+	message.type === borogove.MessageType.MessageCall &&
	2327	+	message.callStatus() === "ringing",
	2328	+	);
	2329	+	if (isProposal) {
	2330	+	proposalStarted();
	2331	+	await proposalStoreCanFinish;
	2332	+	}
	2333	+	if (isRinging) ringStoreStarted = true;
	2334	+	const result = await originalStoreMessages(accountId, messages);
	2335	+	for (const message of messages) {
	2336	+	if (message.type === borogove.MessageType.MessageCall) {
	2337	+	storeOrder.push(message.callStatus());
	2338	+	}
	2339	+	}
	2340	+	if (isRinging) ringStoreFinished();
	2341	+	return result;
	2342	+	};
	2343	+
	2344	+	try {
	2345	+	(client as any).processLiveMessage(
	2346	+	borogove.Stanza.parse(`
	2347	+	<message xmlns="jabber:client" from="${contact}/phone" to="${account}" type="chat" id="proposal-message-id">
	2348	+	<propose xmlns="urn:xmpp:jingle-message:0" id="${sid}">
	2349	+	<description xmlns="urn:xmpp:jingle:apps:rtp:1" media="audio"/>
	2350	+	</propose>
	2351	+	<stanza-id xmlns="urn:xmpp:sid:0" by="${account}" id="race-propose-server-id"/>
	2352	+	</message>
	2353	+	`),
	2354	+	null,
	2355	+	);
	2356	+
	2357	+	await proposalStoreStarted;
	2358	+	// Let the real ring path run while the proposal write is still blocked.
	2359	+	await new Promise((resolve) => setTimeout(resolve, 10));
	2360	+	expect(ringStoreStarted).toBe(false);
	2361	+	releaseProposal();
	2362	+	// Wait for the ring write instead of assuming it finishes in a fixed time.
	2363	+	await ringStoreCanFinish;
	2364	+
	2365	+	expect(storeOrder).toEqual(["propose", "ringing"]);
	2366	+	const stored = await persistence.getMessage(account, contact, null, sid);
	2367	+	expect(stored).not.toBeNull();
	2368	+	expect(stored?.direction).toBe(borogove.MessageDirection.MessageReceived);
	2369	+	expect(stored?.senderId).toBe(contact);
	2370	+	} finally {
	2371	+	releaseProposal();
	2372	+	persistence.storeMessages = originalStoreMessages;
	2373	+	}
	2374	+	});
	2375	+
2293	2376		}
2294	2377
2295	2378		type TestKeyPair = {

borogove

Patchset borogove/call-fixes.2 on 2026-11-09T14:58:54Z · commit a3fc6ef

Patchset `borogove/call-fixes.2` on 2026-11-09T14:58:54Z · commit a3fc6ef