Repository: ergochat/ergo (go)

Issue:
pagination issues with CHATHISTORY
Coming out of discussions with @emersion, there are some problems with Ergo's CHATHISTORY implementation:

1. The history storage layer is decoupled from the presentation layer, but the presentation layer can decide to drop messages. For example, it won't send TAGMSG to clients that don't support `draft/event-playback`, and it won't send HistServ PRIVMSG for JOIN/PART/QUIT to clients that don't support `draft/event-playback` and which have `NS SET replay-joins never`.
2. Client code that requests n messages (via the limit parameter of `CHATHISTORY`), but receives fewer than n messages, may reasonably assume that there is no more history available.
3. Pagination becomes less efficient in the case where there are one or more dropped messages at the edge of the paging window (the client doesn't know to skip over them in its next request).
4. There is an edge case where the paging window is entirely full of dropped messages. This will halt attempts to retrieve history.
5. Finally, HistServ PRIVMSG have a "munged" `msgid` value, but these values are not valid for pagination (they cannot be resolved to timestamps)

(5) is easy (we can change the msgid-to-time resolution code to handle munged msgids). CHATHISTORY should most likely never respect the `replay-joins` setting.

TAGMSG is the hard problem here; it's really problematic to send them to clients that don't support `message-tags` (and it seems unfeasible to substitute some other kind of message), but the alternative is entangling the presentation layer and the storage layer, or telling the storage layer how to skip TAGMSG (which would require a MySQL schema change).

Pull request description:
fix #1676, take 2

Patch:
diff --git a/irc/accounts.go b/irc/accounts.go
index 8f8e8b45..97237523 100644
--- a/irc/accounts.go
+++ b/irc/accounts.go
@@ -2294,7 +2294,6 @@ type ReplayJoinsSetting uint
 const (
 	ReplayJoinsCommandsOnly = iota // replay in HISTORY or CHATHISTORY output
 	ReplayJoinsAlways              // replay in HISTORY, CHATHISTORY, or autoreplay
-	ReplayJoinsNever               // never replay
 )
 
 func replayJoinsSettingFromString(str string) (result ReplayJoinsSetting, err error) {
@@ -2303,8 +2302,6 @@ func replayJoinsSettingFromString(str string) (result ReplayJoinsSetting, err er
 		result = ReplayJoinsCommandsOnly
 	case "always":
 		result = ReplayJoinsAlways
-	case "never":
-		result = ReplayJoinsNever
 	default:
 		err = errInvalidParams
 	}
diff --git a/irc/channel.go b/irc/channel.go
index 6f02aea3..91a82a5a 100644
--- a/irc/channel.go
+++ b/irc/channel.go
@@ -917,7 +917,7 @@ func (channel *Channel) autoReplayHistory(client *Client, rb *ResponseBuffer, sk
 	}
 
 	if hasAutoreplayTimestamps {
-		_, seq, _ := channel.server.GetHistorySequence(channel, client, "", 0)
+		_, seq, _ := channel.server.GetHistorySequence(channel, client, "")
 		if seq != nil {
 			zncMax := channel.server.Config().History.ZNCMax
 			items, _ = seq.Between(history.Selector{Time: start}, history.Selector{Time: end}, zncMax)
@@ -935,7 +935,7 @@ func (channel *Channel) autoReplayHistory(client *Client, rb *ResponseBuffer, sk
 			replayLimit = channel.server.Config().History.AutoreplayOnJoin
 		}
 		if 0 < replayLimit {
-			_, seq, _ := channel.server.GetHistorySequence(channel, client, "", 0)
+			_, seq, _ := channel.server.GetHistorySequence(channel, client, "")
 			if seq != nil {
 				items, _ = seq.Between(history.Selector{}, history.Selector{}, replayLimit)
 			}
@@ -952,7 +952,7 @@ func (channel *Channel) autoReplayHistory(client *Client, rb *ResponseBuffer, sk
 		}
 	}
 	if 0 < numItems {
-		channel.replayHistoryItems(rb, items, true)
+		channel.replayHistoryItems(rb, items, false)
 		rb.Flush(true)
 	}
 }
@@ -1035,7 +1035,7 @@ func (channel *Channel) Part(client *Client, message string, rb *ResponseBuffer)
 	client.server.logger.Debug("channels", fmt.Sprintf("%s left channel %s", details.nick, chname))
 }
 
-func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.Item, autoreplay bool) {
+func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.Item, chathistoryCommand bool) {
 	// send an empty batch if necessary, as per the CHATHISTORY spec
 	chname := channel.Name()
 	client := rb.target
@@ -1043,13 +1043,15 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I
 	extendedJoin := rb.session.capabilities.Has(caps.ExtendedJoin)
 	var playJoinsAsPrivmsg bool
 	if !eventPlayback {
-		switch client.AccountSettings().ReplayJoins {
-		case ReplayJoinsCommandsOnly:
-			playJoinsAsPrivmsg = !autoreplay
-		case ReplayJoinsAlways:
+		if chathistoryCommand {
 			playJoinsAsPrivmsg = true
-		case ReplayJoinsNever:
-			playJoinsAsPrivmsg = false
+		} else {
+			switch client.AccountSettings().ReplayJoins {
+			case ReplayJoinsCommandsOnly:
+				playJoinsAsPrivmsg = false
+			case ReplayJoinsAlways:
+				playJoinsAsPrivmsg = true
+			}
 		}
 	}
 
@@ -1066,6 +1068,9 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I
 		case history.Tagmsg:
 			if eventPlayback {
 				rb.AddSplitMessageFromClient(item.Nick, item.AccountName, item.IsBot, item.Tags, "TAGMSG", chname, item.Message)
+			} else if chathistoryCommand {
+				// #1676, we have to send something here or else it breaks pagination
+				rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, fmt.Sprintf(client.t("%s sent a TAGMSG"), nick))
 			}
 		case history.Join:
 			if eventPlayback {
diff --git a/irc/client.go b/irc/client.go
index d94255bc..6912483c 100644
--- a/irc/client.go
+++ b/irc/client.go
@@ -853,7 +853,7 @@ func (session *Session) Ping() {
 	session.Send(nil, "", "PING", session.client.Nick())
 }
 
-func (client *Client) replayPrivmsgHistory(rb *ResponseBuffer, items []history.Item, target string) {
+func (client *Client) replayPrivmsgHistory(rb *ResponseBuffer, items []history.Item, target string, chathistoryCommand bool) {
 	var batchID string
 	details := client.Details()
 	nick := details.nick
@@ -893,10 +893,15 @@ func (client *Client) replayPrivmsgHistory(rb *ResponseBuffer, items []history.I
 		case history.Tagmsg:
 			if hasEventPlayback && hasTags {
 				command = "TAGMSG"
+			} else if chathistoryCommand {
+				// #1676: send something for TAGMSG; we can't discard it entirely
+				// because it'll break pagination
+				rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", fmt.Sprintf(client.t("%[1]s sent you a TAGMSG"), NUHToNick(item.Nick)))
 			} else {
 				continue
 			}
 		default:
+			// see #1676, this shouldn't happen
 			continue
 		}
 		var tags map[string]string
@@ -1713,7 +1718,7 @@ func (client *Client) listTargets(start, end history.Selector, limit int) (resul
 	var base, extras []history.TargetListing
 	var chcfnames []string
 	for _, channel := range client.Channels() {
-		_, seq, err := client.server.GetHistorySequence(channel, client, "", 0)
+		_, seq, err := client.server.GetHistorySequence(channel, client, "")
 		if seq == nil || err != nil {
 			continue
 		}
@@ -1734,7 +1739,7 @@ func (client *Client) listTargets(start, end history.Selector, limit int) (resul
 		extras = append(extras, persistentExtras...)
 	}
 
-	_, cSeq, err := client.server.GetHistorySequence(nil, client, "", 0)
+	_, cSeq, err := client.server.GetHistorySequence(nil, client, "")
 	if err == nil && cSeq != nil {
 		correspondents, err := cSeq.ListCorrespondents(start, end, limit)
 		if err == nil {
@@ -1758,7 +1763,7 @@ func (client *Client) privmsgsBetween(startTime, endTime time.Time, targetLimit,
 		if strings.HasPrefix(target.CfName, "#") {
 			continue
 		}
-		_, seq, err := client.server.GetHistorySequence(nil, client, target.CfName, 0)
+		_, seq, err := client.server.GetHistorySequence(nil, client, target.CfName)
 		if err == nil && seq != nil {
 			items, err := seq.Between(start, end, messageLimit)
 			if err == nil {
diff --git a/irc/database.go b/irc/database.go
index b4b417f4..bfca9d07 100644
--- a/irc/database.go
+++ b/irc/database.go
@@ -24,7 +24,7 @@ const (
 	// 'version' of the database schema
 	keySchemaVersion = "db.version"
 	// latest schema of the db
-	latestDbSchema = 21
+	latestDbSchema = 22
 
 	keyCloakSecret = "crypto.cloak_secret"
 )
@@ -1059,6 +1059,56 @@ func schemaChangeV20To21(config *Config, tx *buntdb.Tx) error {
 	return nil
 }
 
+// #1676: we used to have ReplayJoinsNever, now it's desupported
+func schemaChangeV21To22(config *Config, tx *buntdb.Tx) error {
+	type accountSettingsv22 struct {
+		AutoreplayLines  *int
+		NickEnforcement  NickEnforcementMethod
+		AllowBouncer     MulticlientAllowedSetting
+		ReplayJoins      ReplayJoinsSetting
+		AlwaysOn         PersistentStatus
+		AutoreplayMissed bool
+		DMHistory        HistoryStatus
+		AutoAway         PersistentStatus
+		Email            string
+	}
+
+	var accounts []string
+	var serializedSettings []string
+	settingsPrefix := "account.settings "
+	tx.AscendGreaterOrEqual("", settingsPrefix, func(key, value string) bool {
+		if !strings.HasPrefix(key, settingsPrefix) {
+			return false
+		}
+		account := strings.TrimPrefix(key, settingsPrefix)
+		if _, err := tx.Get("account.verified " + account); err != nil {
+			return true
+		}
+		var settings accountSettingsv22
+		err := json.Unmarshal([]byte(value), &settings)
+		if err != nil {
+			log.Printf("error (v21-22) processing settings for %s: %v\n", account, err)
+			return true
+		}
+		// if necessary, change ReplayJoinsNever (2) to ReplayJoinsCommandsOnly (0)
+		if settings.ReplayJoins == ReplayJoinsSetting(2) {
+			settings.ReplayJoins = ReplayJoinsSetting(0)
+			if b, err := json.Marshal(settings); err == nil {
+				accounts = append(accounts, account)
+				serializedSettings = append(serializedSettings, string(b))
+			} else {
+				log.Printf("error (v21-22) processing settings for %s: %v\n", account, err)
+			}
+		}
+		return true
+	})
+
+	for i, account := range accounts {
+		tx.Set(settingsPrefix+account, serializedSettings[i], nil)
+	}
+	return nil
+}
+
 func getSchemaChange(initialVersion int) (result SchemaChange, ok bool) {
 	for _, change := range allChanges {
 		if initialVersion == change.InitialVersion {
@@ -1169,4 +1219,9 @@ var allChanges = []SchemaChange{
 		TargetVersion:  21,
 		Changer:        schemaChangeV20To21,
 	},
+	{
+		InitialVersion: 21,
+		TargetVersion:  22,
+		Changer:        schemaChangeV21To22,
+	},
 }
diff --git a/irc/handlers.go b/irc/handlers.go
index c0e42574..1f5982a2 100644
--- a/irc/handlers.go
+++ b/irc/handlers.go
@@ -611,9 +611,9 @@ func chathistoryHandler(server *Server, client *Client, msg ircmsg.Message, rb *
 						target.Time.Format(IRCv3TimestampFormat))
 				}
 			} else if channel != nil {
-				channel.replayHistoryItems(rb, items, false)
+				channel.replayHistoryItems(rb, items, true)
 			} else {
-				client.replayPrivmsgHistory(rb, items, target)
+				client.replayPrivmsgHistory(rb, items, target, true)
 			}
 		}
 	}()
@@ -725,17 +725,7 @@ func chathistoryHandler(server *Server, client *Client, msg ircmsg.Message, rb *
 	if listTargets {
 		targets, err = client.listTargets(start, end, limit)
 	} else {
-		// see #1676; for CHATHISTORY we need to make the paging window as exact as possible,
-		// hence filtering out undisplayable messages on the backend, in order to send a full
-		// paging window if possible
-		var flags history.ExcludeFlags
-		if !rb.session.capabilities.Has(caps.EventPlayback) {
-			flags |= history.ExcludeTagmsg
-		}
-		if client.AccountSettings().ReplayJoins == ReplayJoinsNever {
-			flags |= history.ExcludeJoins
-		}
-		channel, sequence, err = server.GetHistorySequence(nil, client, target, flags)
+		channel, sequence, err = server.GetHistorySequence(nil, client, target)
 		if err != nil || sequence == nil {
 			return
 		}
@@ -1132,9 +1122,9 @@ func historyHandler(server *Server, client *Client, msg ircmsg.Message, rb *Resp
 
 	if len(items) != 0 {
 		if channel != nil {
-			channel.replayHistoryItems(rb, items, false)
+			channel.replayHistoryItems(rb, items, true)
 		} else {
-			client.replayPrivmsgHistory(rb, items, "")
+			client.replayPrivmsgHistory(rb, items, "", true)
 		}
 	}
 	return false
diff --git a/irc/history/history.go b/irc/history/history.go
index 34f46d2d..db3c0d22 100644
--- a/irc/history/history.go
+++ b/irc/history/history.go
@@ -53,17 +53,6 @@ func (item *Item) HasMsgid(msgid string) bool {
 	return item.Message.Msgid == msgid
 }
 
-func (item *Item) IsExcluded(excludeFlags ExcludeFlags) bool {
-	switch item.Type {
-	case Tagmsg:
-		return excludeFlags&ExcludeTagmsg != 0
-	case Join, Part, Quit:
-		return excludeFlags&ExcludeJoins != 0
-	default:
-		return false
-	}
-}
-
 type Predicate func(item *Item) (matches bool)
 
 func Reverse(results []Item) {
@@ -166,7 +155,7 @@ func (list *Buffer) lookup(msgid string) (result Item, found bool) {
 // with an indication of whether the results are complete or are missing items
 // because some of that period was discarded. A zero value of `before` is considered
 // higher than all other times.
-func (list *Buffer) betweenHelper(start, end Selector, cutoff time.Time, pred Predicate, limit int, excludeFlags ExcludeFlags) (results []Item, complete bool, err error) {
+func (list *Buffer) betweenHelper(start, end Selector, cutoff time.Time, pred Predicate, limit int) (results []Item, complete bool, err error) {
 	var ascending bool
 
 	defer func() {
@@ -206,8 +195,7 @@ func (list *Buffer) betweenHelper(start, end Selector, cutoff time.Time, pred Pr
 	satisfies := func(item *Item) bool {
 		return (after.IsZero() || item.Message.Time.After(after)) &&
 			(before.IsZero() || item.Message.Time.Before(before)) &&
-			(pred == nil || pred(item)) &&
-			!item.IsExcluded(excludeFlags)
+			(pred == nil || pred(item))
 	}
 
 	return list.matchInternal(satisfies, ascending, limit), complete, nil
@@ -291,10 +279,9 @@ type bufferSequence struct {
 	list   *Buffer
 	pred   Predicate
 	cutoff time.Time
-	flags  ExcludeFlags
 }
 
-func (list *Buffer) MakeSequence(correspondent string, cutoff time.Time, flags ExcludeFlags) Sequence {
+func (list *Buffer) MakeSequence(correspondent string, cutoff time.Time) Sequence {
 	var pred Predicate
 	if correspondent != "" {
 		pred = func(item *Item) bool {
@@ -305,12 +292,11 @@ func (list *Buffer) MakeSequence(correspondent string, cutoff time.Time, flags E
 		list:   list,
 		pred:   pred,
 		cutoff: cutoff,
-		flags:  flags,
 	}
 }
 
 func (seq *bufferSequence) Between(start, end Selector, limit int) (results []Item, err error) {
-	results, _, err = seq.list.betweenHelper(start, end, seq.cutoff, seq.pred, limit, seq.flags)
+	results, _, err = seq.list.betweenHelper(start, end, seq.cutoff, seq.pred, limit)
 	return
 }
 
@@ -391,7 +377,7 @@ func (list *Buffer) Delete(predicate Predicate) (count int) {
 // latest returns the items most recently added, up to `limit`. If `limit` is 0,
 // it returns all items.
 func (list *Buffer) latest(limit int) (results []Item) {
-	results, _, _ = list.betweenHelper(Selector{}, Selector{}, time.Time{}, nil, limit, 0)
+	results, _, _ = list.betweenHelper(Selector{}, Selector{}, time.Time{}, nil, limit)
 	return
 }
 
diff --git a/irc/history/queries.go b/irc/history/queries.go
index 23d89390..72b1168f 100644
--- a/irc/history/queries.go
+++ b/irc/history/queries.go
@@ -8,13 +8,6 @@ import (
 	"time"
 )
 
-type ExcludeFlags uint
-
-const (
-	ExcludeTagmsg ExcludeFlags = 1 << iota
-	ExcludeJoins
-)
-
 // Selector represents a parameter to a CHATHISTORY command
 type Selector struct {
 	Msgid string
diff --git a/irc/histserv.go b/irc/histserv.go
index 5341fcb1..59149fa4 100644
--- a/irc/histserv.go
+++ b/irc/histserv.go
@@ -199,7 +199,7 @@ func histservPlayHandler(service *ircService, server *Server, client *Client, co
 
 // handles parameter parsing and history queries for /HISTORY and /HISTSERV PLAY
 func easySelectHistory(server *Server, client *Client, params []string) (items []history.Item, channel *Channel, err error) {
-	channel, sequence, err := server.GetHistorySequence(nil, client, params[0], 0)
+	channel, sequence, err := server.GetHistorySequence(nil, client, params[0])
 
 	if sequence == nil || err != nil {
 		return nil, nil, errNoSuchChannel
diff --git a/irc/mysql/history.go b/irc/mysql/history.go
index 1ba81e26..8d78291e 100644
--- a/irc/mysql/history.go
+++ b/irc/mysql/history.go
@@ -40,10 +40,6 @@ const (
 	keySchemaMinorVersion = "db.minorversion"
 	cleanupRowLimit       = 50
 	cleanupPauseTime      = 10 * time.Minute
-
-	// if we don't fill the pagination window due to exclusions,
-	// retry with an expanded window at most this many times
-	maxPaginationRetries = 3
 )
 
 type e struct{}
@@ -1037,18 +1033,9 @@ type mySQLHistorySequence struct {
 	target        string
 	correspondent string
 	cutoff        time.Time
-	excludeFlags  history.ExcludeFlags
 }
 
 func (s *mySQLHistorySequence) Between(start, end history.Selector, limit int) (results []history.Item, err error) {
-	if s.excludeFlags == 0 {
-		return s.baseBetween(start, end, limit)
-	} else {
-		return s.betweenWithRetries(start, end, limit)
-	}
-}
-
-func (s *mySQLHistorySequence) baseBetween(start, end history.Selector, limit int) (results []history.Item, err error) {
 	ctx, cancel := context.WithTimeout(context.Background(), s.mysql.getTimeout())
 	defer cancel()
 
@@ -1071,45 +1058,7 @@ func (s *mySQLHistorySequence) baseBetween(start, end history.Selector, limit in
 	return results, err
 }
 
-func (s *mySQLHistorySequence) betweenWithRetries(start, end history.Selector, limit int) (results []history.Item, err error) {
-	applyExclusions := func(currentResults []history.Item, excludeFlags history.ExcludeFlags, trueLimit int) (filteredResults []history.Item) {
-		filteredResults = make([]history.Item, 0, len(currentResults))
-		for _, item := range currentResults {
-			if !item.IsExcluded(excludeFlags) {
-				filteredResults = append(filteredResults, item)
-			}
-			if len(filteredResults) == trueLimit {
-				break
-			}
-		}
-		return
-	}
-
-	i := 1
-	for {
-		currentLimit := limit * i
-		currentResults, err := s.baseBetween(start, end, currentLimit)
-		if err != nil {
-			return nil, err
-		}
-		results = applyExclusions(currentResults, s.excludeFlags, limit)
-		// we're done in any of these three cases:
-		// (1) we filled the window (2) we ran out of results on the backend (3) we can't retry anymore
-		if len(results) == limit || len(currentResults) < currentLimit || i == maxPaginationRetries {
-			return results, nil
-		}
-		i++
-	}
-}
-
 func (s *mySQLHistorySequence) Around(start history.Selector, limit int) (results []history.Item, err error) {
-	// temporarily clear the exclude flags when running GenericAround, since we don't care about
-	// the exactness of the paging window at all
-	oldExcludeFlags := s.excludeFlags
-	s.excludeFlags = 0
-	defer func() {
-		s.excludeFlags = oldExcludeFlags
-	}()
 	return history.GenericAround(s, start, limit)
 }
 
@@ -1134,12 +1083,11 @@ func (seq *mySQLHistorySequence) Ephemeral() bool {
 	return false
 }
 
-func (mysql *MySQL) MakeSequence(target, correspondent string, cutoff time.Time, excludeFlags history.ExcludeFlags) history.Sequence {
+func (mysql *MySQL) MakeSequence(target, correspondent string, cutoff time.Time) history.Sequence {
 	return &mySQLHistorySequence{
 		target:        target,
 		correspondent: correspondent,
 		mysql:         mysql,
 		cutoff:        cutoff,
-		excludeFlags:  excludeFlags,
 	}
 }
diff --git a/irc/nickserv.go b/irc/nickserv.go
index 3658c8fb..8166fee8 100644
--- a/irc/nickserv.go
+++ b/irc/nickserv.go
@@ -284,8 +284,8 @@ default.`,
 				`$bREPLAY-JOINS$b
 'replay-joins' controls whether replayed channel history will include
 lines for join and part. This provides more information about the context of
-messages, but may be spammy. Your options are 'always', 'never', and the default
-of 'commands-only' (the messages will be replayed in /HISTORY output, but not
+messages, but may be spammy. Your options are 'always' and the default of
+'commands-only' (the messages will be replayed in CHATHISTORY output, but not
 during autoreplay).`,
 				`$bALWAYS-ON$b
 'always-on' controls whether your nickname/identity will remain active
@@ -440,8 +440,6 @@ func displaySetting(service *ircService, settingName string, settings AccountSet
 			service.Notice(rb, client.t("You will see JOINs and PARTs in /HISTORY output, but not in autoreplay"))
 		case ReplayJoinsAlways:
 			service.Notice(rb, client.t("You will see JOINs and PARTs in /HISTORY output and in autoreplay"))
-		case ReplayJoinsNever:
-			service.Notice(rb, client.t("You will not see JOINs and PARTs in /HISTORY output or in autoreplay"))
 		}
 	case "multiclient":
 		if !config.Accounts.Multiclient.Enabled {
diff --git a/irc/server.go b/irc/server.go
index 7a43f0f4..3ac8c38f 100644
--- a/irc/server.go
+++ b/irc/server.go
@@ -862,7 +862,7 @@ func (server *Server) setupListeners(config *Config) (err error) {
 // suitable for ListCorrespondents (i.e., this function is still used to
 // decide whether the ringbuf or mysql is authoritative about the client's
 // message history).
-func (server *Server) GetHistorySequence(providedChannel *Channel, client *Client, query string, excludeFlags history.ExcludeFlags) (channel *Channel, sequence history.Sequence, err error) {
+func (server *Server) GetHistorySequence(providedChannel *Channel, client *Client, query string) (channel *Channel, sequence history.Sequence, err error) {
 	config := server.Config()
 	// 4 cases: {persistent, ephemeral} x {normal, conversation}
 	// with ephemeral history, target is implicit in the choice of `hist`,
@@ -940,9 +940,9 @@ func (server *Server) GetHistorySequence(providedChannel *Channel, client *Clien
 	}
 
 	if hist != nil {
-		sequence = hist.MakeSequence(correspondent, cutoff, excludeFlags)
+		sequence = hist.MakeSequence(correspondent, cutoff)
 	} else if target != "" {
-		sequence = server.historyDB.MakeSequence(target, correspondent, cutoff, excludeFlags)
+		sequence = server.historyDB.MakeSequence(target, correspondent, cutoff)
 	}
 	return
 }
diff --git a/irc/znc.go b/irc/znc.go
index b568d3e7..11fe012c 100644
--- a/irc/znc.go
+++ b/irc/znc.go
@@ -189,14 +189,14 @@ func zncPlaybackPlayHandler(client *Client, command string, params []string, rb
 }
 
 func zncPlayPrivmsgsFrom(client *Client, rb *ResponseBuffer, target string, start, end time.Time) {
-	_, sequence, err := client.server.GetHistorySequence(nil, client, target, 0)
+	_, sequence, err := client.server.GetHistorySequence(nil, client, target)
 	if sequence == nil || err != nil {
 		return
 	}
 	zncMax := client.server.Config().History.ZNCMax
 	items, err := sequence.Between(history.Selector{Time: start}, history.Selector{Time: end}, zncMax)
 	if err == nil && len(items) != 0 {
-		client.replayPrivmsgHistory(rb, items, target)
+		client.replayPrivmsgHistory(rb, items, target, false)
 	}
 }
 
@@ -204,7 +204,7 @@ func zncPlayPrivmsgsFromAll(client *Client, rb *ResponseBuffer, start, end time.
 	zncMax := client.server.Config().History.ZNCMax
 	items, err := client.privmsgsBetween(start, end, maxDMTargetsForAutoplay, zncMax)
 	if err == nil && len(items) != 0 {
-		client.replayPrivmsgHistory(rb, items, "")
+		client.replayPrivmsgHistory(rb, items, "", false)
 	}
 }
 
diff --git a/traditional.yaml b/traditional.yaml
index 948bd182..51b0a2f1 100644
--- a/traditional.yaml
+++ b/traditional.yaml
@@ -935,7 +935,7 @@ history:
         # if `default` is false, store TAGMSG containing any of these tags:
         whitelist:
             - "+draft/react"
-            - "react"
+            - "+react"
 
         # if `default` is true, don't store TAGMSG containing any of these tags:
         #blacklist:


Test patch:
diff --git a/irc/history/history_test.go b/irc/history/history_test.go
index 92720083..18ee965c 100644
--- a/irc/history/history_test.go
+++ b/irc/history/history_test.go
@@ -15,7 +15,7 @@ const (
 )
 
 func betweenTimestamps(buf *Buffer, start, end time.Time, limit int) (result []Item, complete bool) {
-	result, complete, _ = buf.betweenHelper(Selector{Time: start}, Selector{Time: end}, time.Time{}, nil, limit, 0)
+	result, complete, _ = buf.betweenHelper(Selector{Time: start}, Selector{Time: end}, time.Time{}, nil, limit)
 	return
 }
 
@@ -45,7 +45,7 @@ func TestEmptyBuffer(t *testing.T) {
 	})
 	since, complete = betweenTimestamps(buf, pastTime, time.Now(), 0)
 	if len(since) != 1 {
-		t.Errorf("should be able to store items in a nonempty buffer: expected %d, got %d", 1, len(since))
+		t.Error("should be able to store items in a nonempty buffer")
 	}
 	if !complete {
 		t.Error("results should be complete")
diff --git a/irctest b/irctest
index 33f0702c..5e4ae7c9 160000
--- a/irctest
+++ b/irctest
@@ -1,1 +1,1 @@
-Subproject commit 33f0702c260ea716a4ad0f24821a50d44c91fca1
+Subproject commit 5e4ae7c99965801cd91d974637ad344a47b5414f