Add user query extension

[emersion]

cc @slingamn

[slingamn]

Thanks, this looks good, I'll look in more detail later.

One thing that concerns me is that we need some provision for the server autoclosing / autoexpiring these, otherwise the list of opened buffers can grow without bound in the case where the user is not using a client that supports this cap. Maybe a non-normative implementation considerations section?

[progval]

If a PRIVMSG is sent to the user while no user query is opened, the server automatically opens a user query before dispatching the message.

I don't think this is desirable in all cases. Users running /msg ChanServ OP #foobar don't expect a query to open automatically, and especially not on all their clients.

[slingamn]

Services would send NOTICE, right?

[progval]

as a reply, but a PRIVMSG is sent (by the sending user) to the NickServ user.

This paragraph says "user" for both the recipient and the sender, so it's not clear which one is meant here.

[slingamn]

I understood it to refer to the recipient but yeah, some clarification would be useful.

For anything other than *Serv it would probably make sense to open the synced query window on send. Maybe it should be a SHOULD and implementations can decide when to do it (e.g. most implementations would want to exempt *Serv)?

[emersion]

Yeah I definitely meant that a query is opened when the connected user receives a PRIVMSG. See the last example. I'm not super sure how to phrase it though, the server performs the action to open a query, and from the server PoV the recipient doesn't receive the message, the server merely forwards it if a client is connected.

I don't think opening a query when the current user sends a PRIVMSG is desirable, let alone *Serv special-casing. Clients can just QUERYUSER/UNQUERYUSER when the user opens a query with the client UI or /query + /close. Clients will not QUERYUSER on /msg, as such no user query will be opened on the server side as expected.

[slingamn]

Some bikesheddy stuff you can take or leave:

• How about one QUERYUSER command with subcommands + (add), - (remove), and L (list), similar to MONITOR?
• Any thoughts on batching QUERYUSER from the server (particularly if there were an L command for listing?)

I still think this needs an implementation considerations section on limiting data growth.

[emersion]

How about one QUERYUSER command with subcommands + (add), - (remove), and L (list), similar to MONITOR?

I'm not in love with the single-character subcommands, I would prefer explicit words as done in e.g. metadata-2, but that's purely cosmetic.

I think the list subcommand could be a better idea instead of implicitly sending a (potentially) big list upon conn registration: the list can be quite large and implicitly sending replies has been problematic in the past (e.g. no-implicit-names, or the messy JOIN burst management).

I wonder if we need to introduce some kind of paging…? I would like the spec to stay as simple as possible, and in practice the list should be limited in size or else it would be annoying to display in a sidebar UI.

Any thoughts on batching QUERYUSER from the server (particularly if there were an L command for listing?)

Sounds like a good idea either way!

I still think this needs an implementation considerations section on limiting data growth.

Yup, was definitely something I wanted to add. Done!

[emersion]

For classic IRC client UIs like gamja, the list is displayed in the sidebar alongside channels, so the user is likely to clean up old and unused user queries to avoid cluttering the screen too much. For clients like Goguma, the user typically never closes older user queries, since these get naturally pushed back to the back of the list. I've wondered whether we needed to accommodate for this in the spec, but I haven't found anything we could do differently.

[emersion]

I've been wondering whether we should automatically/implicitly add opened user queries to the MONITOR list, to ease client implementations. Clients are very likely interested to get updates from opened user queries just like they get updates from opened channels. But adding some more implicit semantics also makes the whole system more difficult to understand, and the list of opened user queries may grow larger than the MONITOR limit. (FWIW, Goguma only monitors opened user queries interacted with in the last 5 days, up to the MONITOR limit.)

So I think I'd lean on just keeping these two completely separate.

[slingamn]

I'm not in love with the single-character subcommands, I would prefer explicit words as done in e.g. metadata-2, but that's purely cosmetic.

Fine by me :-) I just prefer subcommands over multiple commands, for cleanliness / extensibility.

[slingamn]

I think under a normal workload, data size shouldn't really be a concern and it should be fine to send all open queries as part of (technically after) the registration burst, without pagination.

[emersion]

Switched to a sub-command structure, added a LIST sub-command and a new batch type.

[skizzerz]

Why are the OPEN/CLOSE subcommands fallible? If someone tries to OPEN a query that's already open, or CLOSE one that's already closed, it's effectively a no-op to just send the success state back and it makes the commands idempotent, and therefore easier to manage on the client side. Imagine a desync situation where a client thinks a query isn't open, and tries to open one, but the server believes it's already open. In this case, the client will want to open the query regardless of whether the server says "you already have it open". Giving that "already open" or "already closed" response is imo meaningless and just serves to clutter the spec.

[emersion]

OPEN is typically broadcast to all other connected clients. I wouldn't want a no-op OPEN to be broadcast to all other clients. An OPEN notification is an indication that the query was closed before.

Clients can trivially ignore ALREADY_OPENED errors.

I don't feel strongly about this.

[slingamn]

The current text has:

The draft/user-query capability MAY be negotiated, and affects which messages are sent by the server as specified below.

I would guess that the intent is to control broadcasting of USERQUERY OPEN and USERQUERY CLOSE to other connected sessions, but I don't see that this is mentioned explicitly in the text.

Either way, do you have feelings about making the CAP a MUST in order to use any of the USERQUERY commands? I'm looking for a clear mechanism for the user agent to opt in to userquery tracking, so in the absence of an opt-in, I don't have to track a list of incoming DMs. "Has the user ever requested the CAP from any client?" would be a fairly clean answer to this question. (If that doesn't work, I think I could use "has the user ever requested the CAP, OR sent any USERQUERY command, from any client?")

[emersion]

do you have feelings about making the CAP a MUST in order to use any of the USERQUERY commands?

Oh yeah, for sure. Your reasoning makes sense, and more generally I'm in favor of requiring caps to be enabled before commands are used.

Updated to require cap negotiation and mention broadcasting.

[skizzerz]

batch should also be a requirement for USERQUERY LIST, rather than an option. If omitted, there's no way for the client to know that the list is complete. Additionally, for traffic mitigation, it'd be best if USERQUERY LIST responses could list multiple nicks on a single line. I can otherwise see someone with 100 queries open getting killed by the server's send queue for running the command.

[slingamn]

I can otherwise see someone with 100 queries open getting killed by the server's send queue for running the command.

I'm inclined to see this as a surmountable server implementation detail (it's not an issue in Ergo for example), but if it's very difficult for mainstream ircds to deal with that would be an argument for a mitigation. (On the other hand, mainstream ircds can't implement this specification anyway because it only makes sense in contexts where you have multiple clients per nick.)

[skizzerz]

I'm inclined to see this as a surmountable server implementation detail (it's not an issue in Ergo for example), but if it's very difficult for mainstream ircds to deal with that would be an argument for a mitigation. (On the other hand, mainstream ircds can't implement this specification anyway because it only makes sense in contexts where you have multiple clients per nick.)

Forcing the same sort of async handling that SAFELIST requires would largely just see mainstream ircds not implementing this at all. It is not at all trivial to queue a bunch of lines without killing off the user. Just because ergo is currently the only ircd that is able to implement this currently doesn't mean it'll always stay that way, and it's best not to make assumptions about how easy it is for an ircd to do something based on how ergo is implemented; we really don't need more specs that are practically impossible to implement outside of ergo.

Besides, it's a huge amount of overhead on the wire to do one USERQUERY LIST line per nick when there's plenty of leftover space to combine them. No reason to send 4kb when 1kb would accomplish the same goal, and it produces no meaningful extra client burden (a nested loop to iterate over the nicks in the list). So, the optimization is pretty obviously a win/win imo, even in ergo.

[slingamn]

OK, sounds reasonable to me.

[emersion]

batch should also be a requirement for USERQUERY LIST, rather than an option. If omitted, there's no way for the client to know that the list is complete.

Same with other extensions with optional batching, such as chathistory and extended-isupport. The expectation is that the client will handle messages as they come, much like unsolicited JOIN messages.

Additionally, for traffic mitigation, it'd be best if USERQUERY LIST responses could list multiple nicks on a single line.

I want to keep one nick per line because:

1. That keeps messages extensible via an extended-join-like mechanism. Attaching additional metadata per nick may be desirable in the future.
2. The amount of data is going to be similar to unsolicited JOIN messages: users have a similar amount of opened channels and opened user queries. If the ircd is in a position where it needs something fancy (e.g. async handling) to send the list of opened channels but doesn't need it for opened user queries, then we haven't gained anything.

I can otherwise see someone with 100 queries open getting killed by the server's send queue for running the command.

That's why the spec allows servers to auto-close opened user queries. (Same as the server limiting the amount of opened channels.)

[slingamn]

With the CAP enabled, should you get the equivalent of USERQUERY LIST after the registration burst (along with, e.g. initial JOIN and RPL_NAMREPLY lines)?

[emersion]

For me, LIST was added with the main purpose to not send the list implicitly: #569 (comment)

I don't see why LIST is useful if it's sent upon connection registration.

[slingamn]

Is the source of USERQUERY OPEN or CLOSE always the user themself? If so, what information does it convey?

[emersion]

Right. I modeled it after JOIN, but maybe that doesn't make sense and the prefix should just omitted or the server's. I notice now that MARKREAD doesn't use the user as the prefix.

[slingamn]

Thanks, I'll try leaving it off.

[emersion]

I've switched to server source in the examples. (I always wonder if I should just omit the server source in spec examples, since a missing source means it's coming from the server.)

[slingamn]

My understanding of the state of discussion here is that we concluded that when combined with server-side pruning, this (auto-opening queries) becomes a DoS vector. But I don't remember the state of the discussion for alternative strategies.