Add message redaction #524

progval · 2023-05-26T18:36:28Z

This is #425 with the following changes:

no edits, because it's way more complicated to handle them safely, so IMO it should be deferred to a future spec
targetmsgid is moved to a param (instead of a tag, because it didn't really make sense with a dedicated verb)
added an optional reason (just because I can)
renamed "delete" to "redact" so the cap name doesn't conflict with the other one (and there is some argument esp. in Matrix circles that "redact" better reflects that it's best effort and other clients might not respect it, but I don't really buy it)
explicitly gives a lot of freedom to servers wrt chathistory
new UNKNOWN_MSGID fail tag (because I noticed that's needed while implementing the spec)

Implementations:

Ergo (server): Implement draft/message-redaction ergochat/ergo#2065
Matrix2051 (gateway): Implement IRCv3 draft/message-redaction progval/matrix2051#53
Limnoria (client): Implement IRCv3 draft/message-redaction progval/Limnoria#1538 (sending only)
Unreal (server): Add 'history_delete' method to HistoryBackend unrealircd/unrealircd#253 (new core API) + redact: Initial commit unrealircd/unrealircd-contrib#91 (third-party module)
Goguma (client): https://git.sr.ht/~emersion/goguma/log/redact
IRCCloud

DarthGandalf · 2023-05-26T18:45:22Z

client A sends a message
client B receives the message
client B disconnects
client A sends REDACT
client B reconnects

Client B has no chance of deleting the message even if it supports it. Subject to chathistory, if server sent such REDACT upon reconnect, if the buffer was big enough.

progval · 2023-05-26T18:50:15Z

@DarthGandalf It's unreasonable to make it a requirement; but I mentioned in the "Server implementation considerations" that servers may send it in the chathistory batch when clients re-join (Ergo, Insp, and Unreal already send such a batch for clients without the draft/chathistory cap who aren't bots)

delthas

😋

extensions/message-redaction.md

Co-authored-by: delthas <1863865+delthas@users.noreply.github.com>

extensions/message-redaction.md

DarthGandalf · 2023-05-31T06:15:32Z

extensions/message-redaction.md

+
+This specification enables messages to be deleted.
+Use cases include retracting accidentally sent messages, moderation,
+and removing a [`+draft/react` client tag][], amongst others.


What should happen to existing reactions to the redacted message? And what should happen to attempts to redact such reacts later?

extensions/message-redaction.md

emersion · 2023-05-31T09:07:27Z

extensions/message-redaction.md

+
+* exclude it entirely
+* replace it with the `REDACT` message with the same value as `@msgid` tag
+* add the `REDACT` message to the response (not counting toward message limits)


How does this interact with the event-playback cap?

As in, redacting JOIN/PART/... messages?

My question is basically: are REDACT messages always returned by chathistory regardless of event-playback?

I'd say yes. The motivation for event-playback is that it's hard to deal with channel state (membership, op list, ...) and not necessarily very useful, right? Here this shouldn't be an issue.

Right. I agree.

Does this need to be specified in this spec?

emersion · 2023-05-31T10:52:34Z

Goguma impl (currently untested): https://git.sr.ht/~emersion/goguma/log/redact

As emersion pointed out, it's just too weird

emersion · 2023-06-01T18:33:48Z

extensions/message-redaction.md

+* exclude it entirely
+* replace its second parameter and/or tags with a placeholder and
+  add the `REDACT` message to the response (not counting toward message limits)
+* add the `REDACT` message to the response (not counting toward message limits)


not counting toward message limits

This is at odds with #437 (comment).

Clients have no way to differentiate between a REDACT returned implicitly by chathistory due to another message, or a standalone REDACT message,

Standalone REDACT messages are important to return in the case of a mobile client synchronizing its history at regular intervals via chathistory. Example:

A PRIVMSG is sent at 20:00

A mobile client synchronizes, gets the PRIVMSG via chathistory

A REDACT is sent at 20:30

The mobile client synchronizes again, queries all messages sent strictly after 20:00

The server must return REDACT at that point, or else the client misses it.

Why do clients need to differentiate between the two?

Per the linked discussion above, my clients check whether there is more history to fetch by counting the number of messages. If it's less than the limit, no more messages.

Hm, but now that I've written this, this would only cause the number of messages to exceed the limit, causing needless but harmless chathistory requests in the edge case where there are no more messages. So it should be all fine?

Does a standalone REDACT count towards the limit? I'd say yes. This draws a line between "standalone" and "context" messages.

didn't the linked discussion settle on the empty batch to signify the end?

The discussion hasn't really settled AFAIU, and my clients are still doing the limit check.

Please note, there is an interleaved discussion about what should happen when a channel has chathistory disabled, and this one has settled on empty batches instead of a FAIL. But it's unrelated.

There are two ways to fix this issue that I see:

Add a @chathistory-context tag to all "related"/"context" messages returned by chathistory: chathistory: add chathistory-context tag #526. Clients can ignore these when figuring out the selector for their next query.

Add cursor-based paging to chathistory: chathistory: add cursors #525.

Does a resolution to this issue require changes to the redaction spec, or just to the chathistory spec? i.e. is this a blocker for merge/ratification?

only the chathistory spec IMO

Just spotted this other comment from @emersion that appears unaddressed https://github.com/ircv3/ircv3-specifications/pull/524/files#r1213547049

This PR has become a bit of a nightmare to review lol.

emersion · 2023-06-09T13:52:18Z

Goguma impl has been tested with Ergo.

See: ircv3/ircv3-specifications#524

progval · 2023-11-11T10:29:36Z

@jwheare Anything blocking merge?

progval · 2023-11-17T10:37:43Z

@jwheare ping?

SadieCat · 2023-11-17T13:49:15Z

extensions/message-redaction.md

+
+* `FAIL REDACT INVALID_TARGET the_given_target :You cannot delete messages from the_given_target`
+* `FAIL REDACT REDACT_FORBIDDEN <target> <target-msgid> :You are not authorised to delete this message`
+* `FAIL REDACT REDACT_WINDOW_EXPIRED <target> <target-msgid> <window> :You can no longer edit this message`


How is the window communicated to clients?

For now, it's not. It could be an informational mode though.

It's an easy thing to add later if needed. I don't believe it's worth blocking this PR because of this.

extensions/message-redaction.md

jwheare · 2023-11-27T12:50:49Z

extensions/message-redaction.md

+
+This specification enables messages to be deleted.
+Use cases include retracting accidentally sent messages, moderation,
+and removing a [`+draft/react` client tag][], amongst others.


I definitely think that react needs a way to unreact that doesn't rely on this spec. We can maybe mention that reactions can be REDACTed and this should have the same result, but that probably belongs in the react spec, not this one.

I would rather remove all references to react in this spec, but I can't think of another TAGMSG for the examples section. Original discussion here: https://github.com/ircv3/ircv3-specifications/pull/425/files#r602543189

I think it makes sense to have it both in the intro of this spec, and formally in the react spec

extensions/message-redaction.md

Co-authored-by: James Wheare <james@wheare.org>

It's distracting

Co-authored-by: James Wheare <james@wheare.org>

emersion · 2024-02-09T16:30:52Z

Gentle ping

jwheare · 2024-02-22T22:57:55Z

extensions/message-redaction.md

+* Allowing channel moderators or server admins to delete unwelcome messages from others
+* Specifying a cut-off time after which message edits are no longer allowed
+
+If a message is redacted while a client is not present in a channel, servers may send the `REDACT` command in a `chathistory` batch when it re-joins the channel.


Suggested change

If a message is redacted while a client is not present in a channel, servers may send the `REDACT` command in a `chathistory` batch when it re-joins the channel.

Servers may send the `REDACT` command in a `chathistory` batch.

Removes the implication that servers are tracking whether clients are present in a channel or not.

Should this section be in the normative part of the spec?

The implication was intentional. And the normative already says REDACT can be sent in chathistory batches.

The spec talks about the chathistory spec, not chathistory batches.

ugh. I guess I meant all chathistory batches rather than just chathistory responses

I'm still not sure why this implication is necessary. Which servers keep track of past membership when determining whether to send REDACTs in chathistory?

none of them, but they could if they wanted to

That could apply to a lot of things, what's the motivation for the special case here. Are you thinking of this in the context of e.g. the server can send a REDACT and omit the original PRIVMSG?

yes, so that clients end up showing the same things as if they were in the channel the whole time

extensions/message-redaction.md

Co-authored-by: James Wheare <james@wheare.org>

jwheare · 2024-04-09T09:59:51Z

Merged due to complexity of further review within this PR. Follow up unresolved issues are here #537

progval added 3 commits May 25, 2023 20:44

First draft of message-redaction

6f44ee6

Add UNKNOWN_MSGID fail code

d6d45f6

must ignore the cap's value

b092e88

This was referenced May 26, 2023

Add tests for draft/message-redaction progval/irctest#203

Draft

Implement IRCv3 draft/message-redaction progval/matrix2051#53

Merged

Implement draft/message-redaction ergochat/ergo#2065

Merged

Mention sending REDACT in chathistory batches on join.

0e0f71c

progval mentioned this pull request May 26, 2023

Implement IRCv3 draft/message-redaction progval/Limnoria#1538

Draft

delthas reviewed May 27, 2023

View reviewed changes

extensions/message-redaction.md Outdated Show resolved Hide resolved

extensions/message-redaction.md Outdated Show resolved Hide resolved

extensions/message-redaction.md Outdated Show resolved Hide resolved

progval and others added 7 commits May 27, 2023 08:07

s/DELETE/REDACT/ in examples

323454d

Co-authored-by: delthas <1863865+delthas@users.noreply.github.com>

Add a note about leaking message existence

4efeb02

fix typo in intro

4962974

I can't type

4432a2d

really

15f353d

s/editable/redactable/

6328b52

Mention clients should check the target matches the msgid

6fe3a60

DarthGandalf reviewed May 31, 2023

View reviewed changes

Fix chanop redact example

a70a629

emersion reviewed May 31, 2023

View reviewed changes

extensions/message-redaction.md Outdated Show resolved Hide resolved

emersion reviewed May 31, 2023

View reviewed changes

extensions/message-redaction.md Outdated Show resolved Hide resolved

emersion reviewed May 31, 2023

View reviewed changes

extensions/message-redaction.md Outdated Show resolved Hide resolved

progval added 2 commits May 31, 2023 08:30

Remove another reference to edits

4ba56db

labeled-response isn't required

6b2074b

emersion reviewed May 31, 2023

View reviewed changes

extensions/message-redaction.md Outdated Show resolved Hide resolved

emersion reviewed May 31, 2023

View reviewed changes

Remove "replace it with the REDACT message" option

4e0d316

As emersion pointed out, it's just too weird

emersion reviewed Jun 1, 2023

View reviewed changes

progval mentioned this pull request Jun 4, 2023

Add 'history_delete' method to HistoryBackend unrealircd/unrealircd#253

Merged

delthas added a commit to delthas/discord-ircv3 that referenced this pull request Jun 9, 2023

Implement message deletion / redaction

8b4ad22

See: ircv3/ircv3-specifications#524

emersion mentioned this pull request Jun 14, 2023

chathistory: add chathistory-context tag #526

Open

progval mentioned this pull request Jun 25, 2023

redact: Initial commit unrealircd/unrealircd-contrib#91

Merged

poVoq mentioned this pull request Sep 18, 2023

Message redaction support? bitbot-irc/bitbot#373

Closed

SadieCat reviewed Nov 17, 2023

View reviewed changes

Explicit the types of messages which can be redacted

06d4c3c

jwheare reviewed Nov 27, 2023

View reviewed changes

extensions/message-redaction.md Show resolved Hide resolved

jwheare requested changes Nov 27, 2023

View reviewed changes

progval and others added 9 commits December 1, 2023 13:03

Fix/clarify grammar

0629dd4

Co-authored-by: James Wheare <james@wheare.org>

Remove redundant restriction on msgid grammar

9a97dc1

Clarify PRIVMSG/NOTICE content replacement

0258e52

Remove msgid from REDACT messages in examples

eea9ec4

It's distracting

Fix typo

1680c6b

Clarify when to send REDACT on join

8ab12e9

Co-authored-by: James Wheare <james@wheare.org>

Describe 'reason' parameter

dfc1d8b

Co-authored-by: James Wheare <james@wheare.org>

s/second parameter/content/

89e6a3c

Clarify the position of REDACT messages in chathistory batches

8fb0a99

4e554c4c mentioned this pull request Dec 9, 2023

Message Deletion ircv3/ircv3-ideas#103

Open

Re-add 'Message validation' section

16ccba5

jwheare reviewed Feb 22, 2024

View reviewed changes

extensions/message-redaction.md Outdated Show resolved Hide resolved

fix swapped links

2ef31eb

Co-authored-by: James Wheare <james@wheare.org>

jwheare mentioned this pull request Apr 9, 2024

Unresolved issues with message redaction #537

Open

jwheare merged commit 0a9a4f3 into ircv3:master Apr 9, 2024

	If a message is redacted while a client is not present in a channel, servers may send the `REDACT` command in a `chathistory` batch when it re-joins the channel.
	Servers may send the `REDACT` command in a `chathistory` batch.

Add message redaction #524

Add message redaction #524

Conversation

progval commented May 26, 2023 • edited by jwheare

DarthGandalf commented May 26, 2023

progval commented May 26, 2023

delthas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emersion commented May 31, 2023

Choose a reason for hiding this comment

progval Jun 1, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emersion Jun 2, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emersion commented Jun 9, 2023

progval commented Nov 11, 2023

progval commented Nov 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emersion commented Feb 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwheare commented Apr 9, 2024

progval commented May 26, 2023 •

edited by jwheare

progval Jun 1, 2023 •

edited

emersion Jun 2, 2023 •

edited