borogove

Un-trust/un-bookmark on roster remove rss
created pr with borogove/roster-remove.1 on 2026-10-25T21:36:49Z · by 9e834e0d
cmds
checkout latest patchset: 
ssh pr.pico.sh pull borogove:roster-remove | git am -3
checkout specific patchset revision: 
ssh pr.pico.sh pull borogove:roster-remove [rev] | git am -3
add changes to patch request: 
git format-patch main --stdout | ssh pr.pico.sh borogove:roster-remove
add comment to patch request: 
ssh pr.pico.sh comment borogove:roster-remove "lgtm!"

Patchset borogove/roster-remove.1 on 2026-10-25T21:36:49Z · commit 1858c4f

→ Un-trust/un-bookmark on roster remove
Stephen Paul Weber 2026-10-25T21:35:45Z
Un-trust/un-bookmark on roster remove
Stephen Paul Weber 2026-10-25T21:35:45Z
If the user removes a roster item with another app, we should update our
state to know this Chat is no longer trusted or bookmarked (and also
remove any groups/tags that came from the bookmark, etc).
Semantic diff summary
0 added, 0 modified, 0 signature changed, 0 removed across 0 analyzed files (5 files skipped: unsupported file type)
+5 -2 borogove/Chat.hx #
......@@ -603,11 +603,14 @@ abstract class Chat extends EventEmitter {
603603
604604 @:allow(borogove)
605605 private function updateFromRoster(item: RosterItem) {
606- isBookmarked = true;
606+ final isRemoved = item.subscription == "remove";
607+
608+ isBookmarked = !isRemoved;
607609 setTrusted(item.subscription == "both" || item.subscription == "from");
610+ extensions.removeChildren("group", "jabber:iq:roster");
611+ if (isRemoved) return;
608612 if (item.name != null && item.name != "") displayName = item.name;
609613 if (uiState == Invited) uiState = Open;
610- extensions.removeChildren("group", "jabber:iq:roster");
611614 for (group in item.groups) {
612615 extensions.textTag("group", group, { xmlns: "jabber:iq:roster" });
613616 }
+23 -9 borogove/Client.hx #
......@@ -315,11 +315,13 @@ class Client extends EventEmitter {
315315
316316 final chatsToUpdate = [];
317317 for (item in items) {
318- if (item.subscription != "remove") {
319- final chat = getDirectChat(item.jid, false);
320- chat.updateFromRoster(item);
321- chatsToUpdate.push(cast (chat, Chat));
322- }
318+ final chat = item.subscription == "remove"
319+ ? Util.downcast(getChat(item.jid), DirectChat)
320+ : getDirectChat(item.jid, false);
321+ if (chat == null) continue;
322+
323+ chat.updateFromRoster(item);
324+ chatsToUpdate.push(cast (chat, Chat));
323325 }
324326 persistence.storeChats(this.accountId(), chatsToUpdate);
325327 this.trigger("chats/update", chatsToUpdate);
......@@ -2211,14 +2213,26 @@ class Client extends EventEmitter {
22112213 var rosterGet = new RosterGet(rosterVer);
22122214 rosterGet.onFinished(() -> {
22132215 final chatsToUpdate = [];
2214- // TODO: We should update all chats not in this list as not trusted.
2215- // Special case for when it is empty but there was no <query/> child
2216- // to do nothing is needed also.
2216+ final rosterJids: Map<String, Bool> = new Map();
22172217 for (item in rosterGet.getResult()) {
2218- var chat = getDirectChat(item.jid, false);
2218+ rosterJids[item.jid] = true;
2219+ final chat = getDirectChat(item.jid, false);
22192220 chat.updateFromRoster(item);
22202221 chatsToUpdate.push(cast (chat, Chat));
22212222 }
2223+
2224+ // A versioned response with no query means our cached roster is still
2225+ // current and any modifications will arrive as roster pushes.
2226+ if (rosterGet.hasFullRoster()) {
2227+ for (chat in chats) {
2228+ final directChat = Util.downcast(chat, DirectChat);
2229+ if (directChat == null || rosterJids.exists(directChat.chatId)) continue;
2230+
2231+ directChat.updateFromRoster(new Stanza("item", { jid: directChat.chatId, subscription: "remove" }));
2232+ chatsToUpdate.push(cast (directChat, Chat));
2233+ }
2234+ }
2235+
22222236 if (rosterGet.ver != null) {
22232237 rosterVer = rosterGet.ver;
22242238 persistence.storeLogin(accountId(), stream.clientId ?? jid.resource, displayName(), rosterVer, null);
+10 -0 borogove/queries/RosterGet.hx #
......@@ -31,6 +31,16 @@ class RosterGet extends GenericQuery {
3131 finish();
3232 }
3333
34+ /**
35+ Whether the response contained the full roster.
36+
37+ A versioned response may contain no query at all when the cached roster
38+ is still current; that is different from an empty roster query.
39+ **/
40+ public function hasFullRoster():Bool {
41+ return responseStanza != null && responseStanza.getChild("query", xmlns) != null;
42+ }
43+
3444 public function getResult() {
3545 if (responseStanza == null) {
3646 return [];
+19 -1 borogove/streams/TestStream.hx #
......@@ -8,6 +8,8 @@ import borogove.GenericStream;
88 import borogove.Stanza;
99
1010 class TestStream extends GenericStream {
11+ private final iqHandlers: Map<IqRequestType, Map<String, Stanza->IqResult>> = [IqRequestType.Get => [], IqRequestType.Set => []];
12+
1113 public function register(domain: String, preAuth: Null<String>) {
1214 return Promise.resolve(null);
1315 }
......@@ -22,5 +24,21 @@ class TestStream extends GenericStream {
2224 this.trigger("sendStanza", stanza);
2325 }
2426
25- public function onIq(type:IqRequestType, tag:String, xmlns:String, handler:(Stanza)->IqResult):Void { }
27+ public function onIq(type:IqRequestType, tag:String, xmlns:String, handler:(Stanza)->IqResult):Void {
28+ iqHandlers[type]['{' + xmlns + '}' + tag] = handler;
29+ }
30+
31+ /**
32+ Dispatch an incoming IQ request to the handler registered for it.
33+ **/
34+ public function dispatchIq(stanza:Stanza):IqResult {
35+ final child = stanza.getFirstChild();
36+ if (child == null) return IqNoResult;
37+
38+ final type = stanza.attr.get("type") == "get" ? IqRequestType.Get : IqRequestType.Set;
39+ final handler = iqHandlers[type]['{' + child.attr.get("xmlns") + '}' + child.name];
40+ if (handler == null) return IqNoResult;
41+
42+ return handler(stanza);
43+ }
2644 }
+184 -0 test/TestClient.hx #
......@@ -599,6 +599,190 @@ class TestClient extends utest.Test {
599599 client.stream.onStanza(stanza);
600600 }
601601
602+ public function testLiveRosterRemovalClearsRosterState() {
603+ final persistence = new Dummy();
604+ final client = new Client("test@example.com", persistence);
605+ final chat = client.getDirectChat("bob@example.com");
606+ final stream = Std.downcast(client.stream, borogove.streams.TestStream);
607+
608+ stream.dispatchIq(
609+ new Stanza("iq", { xmlns: "jabber:client", type: "set", from: "example.com" })
610+ .tag("query", { xmlns: "jabber:iq:roster" })
611+ .tag("item", { jid: "bob@example.com", name: "Bob", subscription: "both" })
612+ );
613+
614+ Assert.isTrue(chat.isTrusted());
615+ Assert.isTrue(chat.isBookmarked);
616+ Assert.equals("Bob", chat.getDisplayName());
617+
618+ chat.setTags(["friends"]);
619+ Assert.same(["friends"], chat.getTags());
620+
621+ chat.uiState = Invited;
622+
623+ stream.dispatchIq(
624+ new Stanza("iq", { xmlns: "jabber:client", type: "set", from: "example.com" })
625+ .tag("query", { xmlns: "jabber:iq:roster" })
626+ .tag("item", { jid: "bob@example.com", subscription: "remove" })
627+ );
628+
629+ Assert.isFalse(chat.isTrusted());
630+ Assert.isFalse(chat.isBookmarked);
631+ Assert.same([], chat.getTags());
632+ Assert.equals("Bob", chat.displayName);
633+ Assert.equals(Invited, chat.uiState);
634+ }
635+
636+ public function testLiveRosterRemovalDoesNotCreateChat() {
637+ final persistence = new Dummy();
638+ final client = new Client("test@example.com", persistence);
639+ final stream = Std.downcast(client.stream, borogove.streams.TestStream);
640+
641+ stream.dispatchIq(
642+ new Stanza("iq", { xmlns: "jabber:client", type: "set", from: "example.com" })
643+ .tag("query", { xmlns: "jabber:iq:roster" })
644+ .tag("item", { jid: "nobody@example.com", subscription: "remove" })
645+ );
646+
647+ Assert.isNull(client.getChat("nobody@example.com"));
648+ }
649+
650+ public function testLiveRosterSubscriptionChangeUntrustsChat() {
651+ final persistence = new Dummy();
652+ final client = new Client("test@example.com", persistence);
653+ final chat = client.getDirectChat("bob@example.com");
654+ chat.setTrusted(true);
655+
656+ final stream = Std.downcast(client.stream, borogove.streams.TestStream);
657+ stream.dispatchIq(
658+ new Stanza("iq", { xmlns: "jabber:client", type: "set", from: "example.com" })
659+ .tag("query", { xmlns: "jabber:iq:roster" })
660+ .tag("item", { jid: "bob@example.com", subscription: "to" })
661+ );
662+
663+ Assert.isFalse(chat.isTrusted());
664+ }
665+
666+ public function testRosterGetFullRosterMissingItemClearsRosterState() {
667+ final persistence = new Dummy();
668+ final client = new Client("test@example.com", persistence);
669+ final chat = client.getDirectChat("bob@example.com");
670+ chat.setTrusted(true);
671+ chat.isBookmarked = true;
672+ chat.setTags(["friends"]);
673+ chat.displayName = "Bob";
674+ chat.uiState = Invited;
675+
676+ client.stream.on("sendStanza", (stanza: Stanza) -> {
677+ if (stanza.name == "iq" && stanza.attr.get("type") == "get" && stanza.getChild("query", "jabber:iq:roster") != null) {
678+ client.stream.onStanza(
679+ new Stanza("iq", { xmlns: "jabber:client", type: "result", from: "example.com", id: stanza.attr.get("id") })
680+ .tag("query", { xmlns: "jabber:iq:roster", ver: "ver-1" })
681+ .tag("item", { jid: "carol@example.com", subscription: "both" })
682+ );
683+ return EventHandled;
684+ }
685+ return EventUnhandled;
686+ });
687+
688+ client.rosterGet();
689+
690+ Assert.isFalse(chat.isTrusted());
691+ Assert.isFalse(chat.isBookmarked);
692+ Assert.same([], chat.getTags());
693+ Assert.equals("Bob", chat.displayName);
694+ Assert.equals(Invited, chat.uiState);
695+ }
696+
697+ public function testRosterGetCachedVersionDoesNotClearRosterState() {
698+ final persistence = new Dummy();
699+ final client = new Client("test@example.com", persistence);
700+ final chat = client.getDirectChat("bob@example.com");
701+ chat.setTrusted(true);
702+ chat.isBookmarked = true;
703+ chat.setTags(["friends"]);
704+ chat.displayName = "Bob";
705+ chat.uiState = Invited;
706+
707+ client.stream.on("sendStanza", (stanza: Stanza) -> {
708+ if (stanza.name == "iq" && stanza.attr.get("type") == "get" && stanza.getChild("query", "jabber:iq:roster") != null) {
709+ client.stream.onStanza(
710+ new Stanza("iq", { xmlns: "jabber:client", type: "result", from: "example.com", id: stanza.attr.get("id") })
711+ );
712+ return EventHandled;
713+ }
714+ return EventUnhandled;
715+ });
716+
717+ client.rosterGet();
718+
719+ Assert.isTrue(chat.isTrusted());
720+ Assert.isTrue(chat.isBookmarked);
721+ Assert.same(["friends"], chat.getTags());
722+ Assert.equals("Bob", chat.displayName);
723+ Assert.equals(Invited, chat.uiState);
724+ }
725+
726+ public function testRosterGetInterimRemovePushClearsRosterState() {
727+ final persistence = new Dummy();
728+ final client = new Client("test@example.com", persistence);
729+ final chat = client.getDirectChat("bob@example.com");
730+ chat.setTrusted(true);
731+ chat.isBookmarked = true;
732+ chat.setTags(["friends"]);
733+ chat.displayName = "Bob";
734+ chat.uiState = Invited;
735+
736+ client.stream.on("sendStanza", (stanza: Stanza) -> {
737+ if (stanza.name == "iq" && stanza.attr.get("type") == "get" && stanza.getChild("query", "jabber:iq:roster") != null) {
738+ client.stream.onStanza(
739+ new Stanza("iq", { xmlns: "jabber:client", type: "result", from: "example.com", id: stanza.attr.get("id") })
740+ );
741+ return EventHandled;
742+ }
743+ return EventUnhandled;
744+ });
745+
746+ client.rosterGet();
747+ Assert.isTrue(chat.isTrusted());
748+
749+ final stream = Std.downcast(client.stream, borogove.streams.TestStream);
750+ stream.dispatchIq(
751+ new Stanza("iq", { xmlns: "jabber:client", type: "set", from: "example.com" })
752+ .tag("query", { xmlns: "jabber:iq:roster", ver: "ver-2" })
753+ .tag("item", { jid: "bob@example.com", subscription: "remove" })
754+ );
755+
756+ Assert.isFalse(chat.isTrusted());
757+ Assert.isFalse(chat.isBookmarked);
758+ Assert.same([], chat.getTags());
759+ Assert.equals("Bob", chat.displayName);
760+ Assert.equals(Invited, chat.uiState);
761+ }
762+
763+ public function testRosterGetSubscriptionChangeUntrustsChat() {
764+ final persistence = new Dummy();
765+ final client = new Client("test@example.com", persistence);
766+ final chat = client.getDirectChat("bob@example.com");
767+ chat.setTrusted(true);
768+
769+ client.stream.on("sendStanza", (stanza: Stanza) -> {
770+ if (stanza.name == "iq" && stanza.attr.get("type") == "get" && stanza.getChild("query", "jabber:iq:roster") != null) {
771+ client.stream.onStanza(
772+ new Stanza("iq", { xmlns: "jabber:client", type: "result", from: "example.com", id: stanza.attr.get("id") })
773+ .tag("query", { xmlns: "jabber:iq:roster", ver: "ver-1" })
774+ .tag("item", { jid: "bob@example.com", subscription: "to" })
775+ );
776+ return EventHandled;
777+ }
778+ return EventUnhandled;
779+ });
780+
781+ client.rosterGet();
782+
783+ Assert.isFalse(chat.isTrusted());
784+ }
785+
602786 public function testSortAfterDirectChat() {
603787 final persistence = new Dummy();
604788 final client = new Client("test@example.com", persistence);
Back to top