nandi/frq · 9c1dfe9

nandi/frqpublic⑂ Fork 0

⑂ 9c1dfe9

A sent message showed twice

`sendDraft` appended a local copy so the line appears the moment it is typed,
and `echo-message` is negotiated so the server sends the same line back with a
msgid on it. Both went on screen.

`seenMessage` was never going to catch this, and deliberately: it refuses to
treat our own untagged lines as replays, because a second "ok" from this
client is a real event rather than an echo. The difference it could not see is
`pending` — a line we sent and have not had back yet — which is a field I
added to `Message` for exactly this and then never read.

So `adoptEcho` folds the echo onto the copy already showing instead of
appending it. Oldest first, because the server echoes in the order it
received, so the same text sent twice adopts onto the earlier copy.

The duplicate was the visible half. The other half is that adopting is how
this client learns the msgid of something it said — which is the whole reason
`echo-message` is in `wantedCaps` — and without it a reaction or a reply aimed
at our own line had nothing to name.

`just test live` now sends a line and counts how many times it comes back on
screen, so the next version of this is caught by a machine rather than by you
looking at it:

    ✓ sent line appears once

340 Nim tests, 7 of them on this.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

nandi committed 2026-09-19T08:08:19-07:00 Browse files

9c1dfe9 parent: 9bb81a1

modified dart/frq_core/tool/live_ui.dart +20 -0 Viewed

@@ -92,6 +92,26 @@ Future<void> main(List<String> args) async {
92	92	walk(tree);
93	93	print('✓ tags in use: ${(tags.toList()..sort()).join(", ")}');
94	94
	95	+ // Send a line, and count how many times it comes back on screen.
	96	+ //
	97	+ // echo-message is negotiated, so the server returns every line this client
	98	+ // sends — and showing both that and the local copy is what put every sent
	99	+ // message on screen twice.
	100	+ final marker = 'frq echo check ${DateTime.now().millisecondsSinceEpoch}';
	101	+ core.dispatch('draft.change', marker);
	102	+ core.dispatch('send');
	103	+ await Future<void>.delayed(const Duration(seconds: 3));
	104	+ tree = core.poll();
	105	+ final copies = find(tree, 'text')
	106	+ .where((e) => e.prop('text', '') == marker)
	107	+ .length;
	108	+ if (copies == 1) {
	109	+ print('✓ sent line appears once');
	110	+ } else {
	111	+ print('✗ sent line appears $copies times');
	112	+ exit(1);
	113	+ }
	114	+
95	115	core.dispatch('disconnect');
96	116	print('✓ disconnected');
97	117	}

    walk(tree);
    print('✓ tags in use: ${(tags.toList()..sort()).join(", ")}');
++  // Send a line, and count how many times it comes back on screen.
++  //
++  // echo-message is negotiated, so the server returns every line this client
++  // sends — and showing both that and the local copy is what put every sent
++  // message on screen twice.
++  final marker = 'frq echo check ${DateTime.now().millisecondsSinceEpoch}';
++  core.dispatch('draft.change', marker);
++  core.dispatch('send');
++  await Future<void>.delayed(const Duration(seconds: 3));
++  tree = core.poll();
++  final copies = find(tree, 'text')
++      .where((e) => e.prop('text', '') == marker)
++      .length;
++  if (copies == 1) {
++    print('✓ sent line appears once');
++  } else {
++    print('✗ sent line appears $copies times');
++    exit(1);
++  }
++
    core.dispatch('disconnect');
    print('✓ disconnected');
  }

modified nim/src/frq/reducer.nim +10 -0 Viewed

@@ -432,6 +432,16 @@ proc drain*() =
432	432	# A message to us rather than to a channel belongs in a buffer named
433	433	# for the sender: the target is our own nick and is nobody's room.
434	434	let room = if target.startsWith("#"): target else: who
	435	+ # Our own line coming back is the copy we already showed, with the
	436	+ # msgid the server gave it — not a new message.
	437	+ if who == app.formNick and app.rooms.hasKey(room):
	438	+ var r = app.rooms[room]
	439	+ let adopted = r.adoptEcho(who, p.params[^1], msgid, at,
	440	+ if p.hasAccount: p.account else: "")
	441	+ if adopted:
	442	+ app.rooms[room] = r
	443	+ continue
	444	+
435	445	var m = Message(id: msgid, frm: who, text: p.params[^1], at: at)
436	446	if p.hasAccount: m.account = p.account
437	447	# The picture link out of the text, which is what draws the inline

          # A message to us rather than to a channel belongs in a buffer named
          # for the sender: the target is our own nick and is nobody's room.
          let room = if target.startsWith("#"): target else: who
++        # Our own line coming back is the copy we already showed, with the
++        # msgid the server gave it — not a new message.
++        if who == app.formNick and app.rooms.hasKey(room):
++          var r = app.rooms[room]
++          let adopted = r.adoptEcho(who, p.params[^1], msgid, at,
++                                    if p.hasAccount: p.account else: "")
++          if adopted:
++            app.rooms[room] = r
++            continue
++
          var m = Message(id: msgid, frm: who, text: p.params[^1], at: at)
          if p.hasAccount: m.account = p.account
          # The picture link out of the text, which is what draws the inline

modified nim/src/frq/rooms.nim +27 -0 Viewed

@@ -160,6 +160,33 @@ func afterMarker*(ch: Room): seq[Message] =
160	160	return if i + 1 <= ch.messages.high: ch.messages[i + 1 .. ^1] else: @[]
161	161	ch.messages.filterIt(it.at > ch.lastReadAt)
162	162
	163	+func adoptEcho*(r: var Room, frm, text, id: string, at: int64,
	164	+ account: string): bool =
	165	+ ## Our own line, coming back from the server, folded onto the copy we
	166	+ ## already showed.
	167	+ ##
	168	+ ## `echo-message` is negotiated, so every line this client sends arrives
	169	+ ## again with a msgid on it — which is the point of asking for the cap, and
	170	+ ## is the only way this client learns what the server called something it
	171	+ ## said. Appending it is what showed every sent message twice.
	172	+ ##
	173	+ ## `seenMessage` deliberately will not catch this: it refuses to treat our
	174	+ ## own untagged lines as replays, because a second "ok" from this client is
	175	+ ## a real event rather than an echo. The difference is `pending` — a line we
	176	+ ## sent and have not seen back yet — and only a pending one is adopted.
	177	+ ##
	178	+ ## Oldest first, because the server echoes in the order it received, so the
	179	+ ## same text sent twice adopts onto the earlier copy.
	180	+ for i in 0 ..< r.messages.len:
	181	+ if r.messages[i].pending and r.messages[i].id.len == 0 and
	182	+ r.messages[i].frm == frm and r.messages[i].text == text:
	183	+ r.messages[i].id = id
	184	+ r.messages[i].pending = false
	185	+ if at > 0: r.messages[i].at = at
	186	+ if account.len > 0: r.messages[i].account = account
	187	+ return true
	188	+ false
	189	+
163	190	func mentionsMe*(m: Message, me: string): bool =
164	191	## Whether a line is addressed at the reader by name. Our own lines do not
165	192	## count — saying your own nick is not being called.

          return if i + 1 <= ch.messages.high: ch.messages[i + 1 .. ^1] else: @[]
    ch.messages.filterIt(it.at > ch.lastReadAt)
++func adoptEcho*(r: var Room, frm, text, id: string, at: int64,
++                account: string): bool =
++  ## Our own line, coming back from the server, folded onto the copy we
++  ## already showed.
++  ##
++  ## `echo-message` is negotiated, so every line this client sends arrives
++  ## again with a msgid on it — which is the point of asking for the cap, and
++  ## is the only way this client learns what the server called something it
++  ## said. Appending it is what showed every sent message twice.
++  ##
++  ## `seenMessage` deliberately will not catch this: it refuses to treat our
++  ## own untagged lines as replays, because a second "ok" from this client is
++  ## a real event rather than an echo. The difference is `pending` — a line we
++  ## sent and have not seen back yet — and only a pending one is adopted.
++  ##
++  ## Oldest first, because the server echoes in the order it received, so the
++  ## same text sent twice adopts onto the earlier copy.
++  for i in 0 ..< r.messages.len:
++    if r.messages[i].pending and r.messages[i].id.len == 0 and
++       r.messages[i].frm == frm and r.messages[i].text == text:
++      r.messages[i].id = id
++      r.messages[i].pending = false
++      if at > 0: r.messages[i].at = at
++      if account.len > 0: r.messages[i].account = account
++      return true
++  false
++
  func mentionsMe*(m: Message, me: string): bool =
    ## Whether a line is addressed at the reader by name. Our own lines do not
    ## count — saying your own nick is not being called.

modified nim/tests/trooms.nim +48 -0 Viewed

@@ -146,3 +146,51 @@ suite "recentEverywhere":
146	146	let got = recentEverywhere(chans, "")
147	147	check got.anyIt(it.text.startsWith("#a"))
148	148	check got.len <= overviewLimit
	149	+
	150	+suite "adoptEcho":
	151	+ setup:
	152	+ var r = initRoom("#test")
	153	+ r.messages = @[msg("alice", "hello", at = 100, id = "1")]
	154	+ # What `sendDraft` leaves behind: shown at once, no msgid yet.
	155	+ r.messages.add Message(frm: "me", text: "hi there", at: 200,
	156	+ localId: "local-1", pending: true)
	157	+
	158	+ test "our own line coming back folds onto the copy we showed":
	159	+ # This is the bug: without it every sent message appeared twice, because
	160	+ # echo-message is negotiated and the server sends it back with an id.
	161	+ check r.adoptEcho("me", "hi there", "srv-9", 250, "did:plc:me")
	162	+ check r.messages.len == 2
	163	+ check r.messages[1].id == "srv-9"
	164	+ check not r.messages[1].pending
	165	+
	166	+ test "and that is how the client learns the msgid":
	167	+ # Which is the whole point of asking for the cap: a reaction or a reply
	168	+ # aimed at our own line has nothing to name until this happens.
	169	+ discard r.adoptEcho("me", "hi there", "srv-9", 250, "")
	170	+ check r.messages[1].answersTo("srv-9")
	171	+ check rowId(r.messages[1]) == "srv-9"
	172	+
	173	+ test "the server's timestamp wins over ours":
	174	+ discard r.adoptEcho("me", "hi there", "srv-9", 250, "")
	175	+ check r.messages[1].at == 250
	176	+
	177	+ test "a line that is not ours is not adopted":
	178	+ check not r.adoptEcho("alice", "hello", "srv-9", 250, "")
	179	+
	180	+ test "nor is one we have already seen back":
	181	+ check r.adoptEcho("me", "hi there", "srv-9", 250, "")
	182	+ # A second "hi there" from this client is a real event, not an echo.
	183	+ check not r.adoptEcho("me", "hi there", "srv-10", 260, "")
	184	+
	185	+ test "the same text sent twice adopts oldest first":
	186	+ # The server echoes in the order it received.
	187	+ r.messages.add Message(frm: "me", text: "hi there", at: 300,
	188	+ localId: "local-2", pending: true)
	189	+ check r.adoptEcho("me", "hi there", "srv-A", 310, "")
	190	+ check r.messages[1].id == "srv-A"
	191	+ check r.messages[2].id == ""
	192	+ check r.adoptEcho("me", "hi there", "srv-B", 320, "")
	193	+ check r.messages[2].id == "srv-B"
	194	+
	195	+ test "different text is not adopted":
	196	+ check not r.adoptEcho("me", "something else", "srv-9", 250, "")

      let got = recentEverywhere(chans, "")
      check got.anyIt(it.text.startsWith("#a"))
      check got.len <= overviewLimit
++
++suite "adoptEcho":
++  setup:
++    var r = initRoom("#test")
++    r.messages = @[msg("alice", "hello", at = 100, id = "1")]
++    # What `sendDraft` leaves behind: shown at once, no msgid yet.
++    r.messages.add Message(frm: "me", text: "hi there", at: 200,
++                           localId: "local-1", pending: true)
++
++  test "our own line coming back folds onto the copy we showed":
++    # This is the bug: without it every sent message appeared twice, because
++    # echo-message is negotiated and the server sends it back with an id.
++    check r.adoptEcho("me", "hi there", "srv-9", 250, "did:plc:me")
++    check r.messages.len == 2
++    check r.messages[1].id == "srv-9"
++    check not r.messages[1].pending
++
++  test "and that is how the client learns the msgid":
++    # Which is the whole point of asking for the cap: a reaction or a reply
++    # aimed at our own line has nothing to name until this happens.
++    discard r.adoptEcho("me", "hi there", "srv-9", 250, "")
++    check r.messages[1].answersTo("srv-9")
++    check rowId(r.messages[1]) == "srv-9"
++
++  test "the server's timestamp wins over ours":
++    discard r.adoptEcho("me", "hi there", "srv-9", 250, "")
++    check r.messages[1].at == 250
++
++  test "a line that is not ours is not adopted":
++    check not r.adoptEcho("alice", "hello", "srv-9", 250, "")
++
++  test "nor is one we have already seen back":
++    check r.adoptEcho("me", "hi there", "srv-9", 250, "")
++    # A second "hi there" from this client is a real event, not an echo.
++    check not r.adoptEcho("me", "hi there", "srv-10", 260, "")
++
++  test "the same text sent twice adopts oldest first":
++    # The server echoes in the order it received.
++    r.messages.add Message(frm: "me", text: "hi there", at: 300,
++                           localId: "local-2", pending: true)
++    check r.adoptEcho("me", "hi there", "srv-A", 310, "")
++    check r.messages[1].id == "srv-A"
++    check r.messages[2].id == ""
++    check r.adoptEcho("me", "hi there", "srv-B", 320, "")
++    check r.messages[2].id == "srv-B"
++
++  test "different text is not adopted":
++    check not r.adoptEcho("me", "something else", "srv-9", 250, "")