From a1db99da680d85e9f1cd58628e93f5a4a0e34d62 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Fri, 7 Aug 2026 14:03:08 -0500 Subject: [PATCH 1/5] Add the state and credential halves of the contract Nine verbs, and chat_poll() stops handing its client back. State (chat_channels, chat_history, chat_pending, chat_mark_read). chat_poll() answers "what changed since my cursor"; a process that just started has no useful cursor and needs "what is true now". chat_pending() is a separate verb rather than a mode of chat_poll() because overloading the cursor would make "start from nothing" and "tell me what is standing" the same call -- and a client that asked for pending invitations and thereby reset its read position would replay every channel it is in. chat_history() returns oldest-first whatever direction the platform paged in. Matrix and Slack both hand back newest-first and every consumer flips it; one flip in the adapter beats one per consumer, and a reversed transcript reads plausibly enough that nothing errors. Credentials (chat_matrix_config, chat_config_save, chat_matrix_configure, chat_relogin, chat_set_identity). chat_config keeps the application's own fields -- corteza stores its bots list and operator policy in the same file -- and carries app and path as attributes rather than fields, so neither is written back into the file as though it were a credential. print() shows field names and never values. chat_set_identity() is what unblocks 1f. The Matrix rename can rotate the access token underneath the caller, and a consumer that made that call itself had to notice and get the new token back into its client, which it did through the file they happened to share. Behind the contract the rotation lands in the client that performed it. So chat_poll() no longer returns $client. The cursor still reports the post-sync token; the refreshed credentials live on the client. Also fixes a hole the new verbs exposed. Every mx.api-backed method passed mx_client_session(client$env$mx) straight into its seam, and R made that a promise -- no seam double reads its session argument, so the session was never built and the test fixture had no token in it at all. The methods force it now, which is what production does the instant the real mx.api function touches it. --- DESCRIPTION | 2 +- NAMESPACE | 31 ++++ R/contract.R | 183 +++++++++++++++++++++ R/irc.R | 29 +++- R/loopback.R | 32 ++++ R/matrix-config.R | 228 +++++++++++++++++++++++++++ R/matrix.R | 161 +++++++++++++++++-- R/slack.R | 96 +++++++++++ inst/tinytest/test_matrix.R | 209 ++++++++++++++++++++++-- inst/tinytest/test_matrix_mxclient.R | 112 +++++++++++++ man/chat_channels.Rd | 24 +++ man/chat_config.Rd | 25 +++ man/chat_config_save.Rd | 27 ++++ man/chat_history.Rd | 47 ++++++ man/chat_mark_read.Rd | 32 ++++ man/chat_matrix.Rd | 23 ++- man/chat_matrix_config.Rd | 41 +++++ man/chat_matrix_config_path.Rd | 25 +++ man/chat_matrix_configure.Rd | 49 ++++++ man/chat_pending.Rd | 35 ++++ man/chat_relogin.Rd | 26 +++ man/chat_set_identity.Rd | 33 ++++ 22 files changed, 1442 insertions(+), 28 deletions(-) create mode 100644 R/matrix-config.R create mode 100644 man/chat_channels.Rd create mode 100644 man/chat_config.Rd create mode 100644 man/chat_config_save.Rd create mode 100644 man/chat_history.Rd create mode 100644 man/chat_mark_read.Rd create mode 100644 man/chat_matrix_config.Rd create mode 100644 man/chat_matrix_config_path.Rd create mode 100644 man/chat_matrix_configure.Rd create mode 100644 man/chat_pending.Rd create mode 100644 man/chat_relogin.Rd create mode 100644 man/chat_set_identity.Rd diff --git a/DESCRIPTION b/DESCRIPTION index 6410032..d62866e 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: chat.api Type: Package Title: Transport-Agnostic Chat Contract for R Agents -Version: 0.0.1.15 +Version: 0.0.1.16 Date: 2026-08-05 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), diff --git a/NAMESPACE b/NAMESPACE index ce48683..55aa2ae 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -3,20 +3,31 @@ export(chat_addressed) export(chat_capabilities) export(chat_channel_info) +export(chat_channels) +export(chat_config) +export(chat_config_save) export(chat_disconnect) +export(chat_history) export(chat_identity) export(chat_invite) export(chat_irc) export(chat_join) export(chat_loopback) +export(chat_mark_read) export(chat_matrix) +export(chat_matrix_config) +export(chat_matrix_config_path) +export(chat_matrix_configure) export(chat_members) export(chat_message) +export(chat_pending) export(chat_poll) export(chat_react) export(chat_reaction) +export(chat_relogin) export(chat_resolve) export(chat_send) +export(chat_set_identity) export(chat_slack) export(chat_typing) export(chat_whoami) @@ -32,14 +43,27 @@ S3method(chat_capabilities,chat_slack) S3method(chat_channel_info,chat_matrix) S3method(chat_channel_info,chat_slack) S3method(chat_channel_info,default) +S3method(chat_channels,chat_loopback) +S3method(chat_channels,chat_matrix) +S3method(chat_channels,chat_slack) +S3method(chat_channels,default) S3method(chat_disconnect,chat_irc) S3method(chat_disconnect,default) +S3method(chat_history,chat_loopback) +S3method(chat_history,chat_matrix) +S3method(chat_history,chat_slack) +S3method(chat_history,default) S3method(chat_join,chat_matrix) S3method(chat_join,chat_slack) S3method(chat_join,default) +S3method(chat_mark_read,chat_matrix) +S3method(chat_mark_read,chat_slack) +S3method(chat_mark_read,default) S3method(chat_members,chat_matrix) S3method(chat_members,chat_slack) S3method(chat_members,default) +S3method(chat_pending,chat_matrix) +S3method(chat_pending,default) S3method(chat_poll,chat_irc) S3method(chat_poll,chat_loopback) S3method(chat_poll,chat_matrix) @@ -47,6 +71,8 @@ S3method(chat_poll,chat_slack) S3method(chat_react,chat_matrix) S3method(chat_react,chat_slack) S3method(chat_react,default) +S3method(chat_relogin,chat_matrix) +S3method(chat_relogin,default) S3method(chat_resolve,chat_irc) S3method(chat_resolve,chat_loopback) S3method(chat_resolve,chat_matrix) @@ -55,6 +81,10 @@ S3method(chat_send,chat_irc) S3method(chat_send,chat_loopback) S3method(chat_send,chat_matrix) S3method(chat_send,chat_slack) +S3method(chat_set_identity,chat_irc) +S3method(chat_set_identity,chat_matrix) +S3method(chat_set_identity,chat_slack) +S3method(chat_set_identity,default) S3method(chat_typing,chat_matrix) S3method(chat_typing,default) S3method(chat_whoami,chat_irc) @@ -62,6 +92,7 @@ S3method(chat_whoami,chat_loopback) S3method(chat_whoami,chat_matrix) S3method(chat_whoami,chat_slack) S3method(chat_whoami,default) +S3method(print,chat_config) S3method(print,chat_identity) S3method(print,chat_invite) S3method(print,chat_message) diff --git a/R/contract.R b/R/contract.R index c18b85f..6fb301d 100644 --- a/R/contract.R +++ b/R/contract.R @@ -537,3 +537,186 @@ identity_mentioned <- function(id, message) { escape_rx <- function(x) { gsub("([][{}()+*^$|\\\\?.])", "\\\\\\1", x) } + +#' List the channels this client is in +#' +#' The state half of the contract. \code{\link{chat_poll}} answers "what +#' changed since my cursor"; this and its siblings answer "what is true +#' now", which is the question a process asks when it starts up with no +#' useful cursor at all. +#' +#' @param client A \code{chat_client}. +#' @param ... Adapter-specific options. +#' @return Character vector of channel identifiers. +#' @examples +#' chat_channels(chat_loopback()) +#' @export +chat_channels <- function(client, ...) { + UseMethod("chat_channels") +} + +#' @export +chat_channels.default <- function(client, ...) { + stop("chat_channels() is not supported by this adapter (", + paste(class(client), collapse = "/"), + "). Check chat_capabilities()$channels.", call. = FALSE) +} + +#' Read a channel's recent messages +#' +#' Independent of the poll cursor: a restarted process uses this to +#' recover the context it lost, and asking for it must not move the +#' cursor or consume anything. +#' +#' @param client A \code{chat_client}. +#' @param channel Channel/room identifier. +#' @param limit Maximum messages to return. +#' @param before Return messages older than this message id, for paging +#' backwards; NULL starts from the most recent. +#' @param ... Adapter-specific options. +#' @return A list of \code{\link{chat_message}}, oldest first. +#' +#' @section Order: +#' Chronological, oldest first, whatever the platform's native direction +#' is. Matrix \code{dir = "b"} and Slack \code{conversations.history} +#' both hand back newest-first and every consumer replaying history into +#' a transcript has to flip it. One flip in the adapter beats one per +#' consumer, and a consumer that gets it wrong produces a transcript +#' that reads backwards without erroring. +#' +#' @section Overlap with chat_poll: +#' The same message can arrive from both, and adapters must return the +#' same \code{id} for it either way. That id is the only thing a consumer +#' has to deduplicate on -- a startup backfill and the first poll after +#' it routinely cover the same events. +#' @examples +#' cl <- chat_loopback() +#' chat_send(cl, "general", "hello") +#' chat_history(cl, "general") +#' @export +chat_history <- function(client, channel, limit = 50L, before = NULL, ...) { + UseMethod("chat_history") +} + +#' @export +chat_history.default <- function(client, channel, limit = 50L, before = NULL, + ...) { + stop("chat_history() is not supported by this adapter (", + paste(class(client), collapse = "/"), + "). Check chat_capabilities()$history.", call. = FALSE) +} + +#' Read standing state that is not tied to a cursor +#' +#' Today: pending invitations. \code{\link{chat_poll}} reports an +#' invitation when it arrives, which is no help to a client that was not +#' running at the time -- and some homeservers only report invites newer +#' than the \code{since} token, so the poll loop never sees them again. +#' +#' This is a separate verb rather than a mode of \code{\link{chat_poll}} +#' deliberately. Overloading the cursor would make "start from nothing" +#' and "tell me what is standing" the same call, and a client that asked +#' for pending invitations and thereby reset its read position would +#' replay every channel it is in. +#' +#' @param client A \code{chat_client}. +#' @param ... Adapter-specific options. +#' @return A list with \code{invites}, a list of \code{\link{chat_invite}}. +#' @examples +#' \dontrun{ +#' pending <- chat_pending(client) +#' for (iv in pending$invites) chat_join(client, iv$channel) +#' } +#' @export +chat_pending <- function(client, ...) { + UseMethod("chat_pending") +} + +#' @export +chat_pending.default <- function(client, ...) { + stop("chat_pending() is not supported by this adapter (", + paste(class(client), collapse = "/"), + "). Check chat_capabilities()$pending.", call. = FALSE) +} + +#' Mark a message as read +#' +#' The default is a quiet FALSE, on \code{\link{chat_typing}}'s +#' reasoning rather than \code{\link{chat_react}}'s: a read marker that +#' does not appear costs a human a little context about what the bot has +#' seen, and nothing more. Nobody is waiting on it the way they wait on +#' an acknowledgement. +#' +#' Write-only. Reading other participants' read state is a much larger +#' surface -- per-user, per-device, and absent entirely on some +#' platforms -- and no consumer needs it yet. +#' +#' @param client A \code{chat_client}. +#' @param channel Channel/room identifier. +#' @param message_id The message to mark read, and everything before it. +#' @param ... Adapter-specific options. +#' @return TRUE if the marker was sent, FALSE otherwise, invisibly. +#' @export +chat_mark_read <- function(client, channel, message_id, ...) { + UseMethod("chat_mark_read") +} + +#' @export +chat_mark_read.default <- function(client, channel, message_id, ...) { + invisible(FALSE) +} + +#' Set this client's persistent identity +#' +#' The account's own display name, as everyone in every channel sees it +#' until it is changed again. Distinct from \code{\link{chat_send}}'s +#' \code{identity} argument, which decorates a single message on +#' platforms that allow it. +#' +#' Owning this matters beyond tidiness. On Matrix the rename is an +#' authenticated call that can rotate the access token underneath the +#' caller, and a consumer that made that call itself had to notice the +#' rotation and get the new token back into its client -- usually via +#' whatever file both of them happened to share. Behind the contract the +#' rotation lands in the client that performed it, and nothing outside +#' has to know it happened. +#' +#' @param client A \code{chat_client}. +#' @param display New display name. +#' @param ... Adapter-specific options. +#' @return TRUE if the identity was changed, invisibly. +#' @export +chat_set_identity <- function(client, display, ...) { + UseMethod("chat_set_identity") +} + +#' @export +chat_set_identity.default <- function(client, display, ...) { + stop("chat_set_identity() is not supported by this adapter (", + paste(class(client), collapse = "/"), + "). Check chat_capabilities()$set_identity.", call. = FALSE) +} + +#' Refresh this client's credentials +#' +#' Forces the re-authentication that adapters otherwise perform on +#' demand. The refreshed credentials stay inside the client. +#' +#' The default throws rather than returning quietly. "I could not +#' refresh" and "there was nothing to refresh" look identical to a +#' caller that gets FALSE, and the first means the next call will fail +#' with a stale token. +#' +#' @param client A \code{chat_client}. +#' @param ... Adapter-specific options. +#' @return TRUE, invisibly. +#' @export +chat_relogin <- function(client, ...) { + UseMethod("chat_relogin") +} + +#' @export +chat_relogin.default <- function(client, ...) { + stop("chat_relogin() is not supported by this adapter (", + paste(class(client), collapse = "/"), ").", call. = FALSE) +} diff --git a/R/irc.R b/R/irc.R index f580fed..cc9bc9b 100644 --- a/R/irc.R +++ b/R/irc.R @@ -126,6 +126,8 @@ chat_capabilities.chat_irc <- function(client, ...) { list(threads = FALSE, thread_replies = FALSE, edits = FALSE, reactions = FALSE, reaction_events = FALSE, channel_info = FALSE, members = FALSE, invites = FALSE, join = FALSE, whoami = TRUE, + channels = FALSE, history = FALSE, pending = FALSE, + mark_read = FALSE, set_identity = TRUE, relogin = FALSE, files = FALSE, typing = FALSE, e2ee = FALSE, identity_override = FALSE, markup_dialects = "plain", max_message_bytes = 400L) @@ -133,11 +135,16 @@ chat_capabilities.chat_irc <- function(client, ...) { #' @export chat_whoami.chat_irc <- function(client, ...) { - # The nick this client sent in its NICK line. A server that refused - # it and assigned another (collision, or a nick longer than the - # server allows) has not been read back here -- the 001 welcome - # carries the real one, and this adapter does not parse it yet. - chat_identity(client$nick) + # env first: a chat_set_identity() NICK change during this session + # writes there, and the list field is only what this client was + # constructed with. Reading the constructor's field would keep + # reporting the old nick for the life of the process, so the bot + # would stop recognising its own name. + # + # A server that refused the nick and assigned another (collision, or + # one longer than it allows) is invisible either way: the 001 + # welcome carries the real one and this adapter does not parse it. + chat_identity(client$env$nick %||% client$nick) } # What may sit next to a nick without being part of it. IRC nicks are @@ -170,3 +177,15 @@ chat_disconnect.chat_irc <- function(client, ...) { tryCatch(close(client$env$con), error = function(e) NULL) invisible(TRUE) } + +#' @export +chat_set_identity.chat_irc <- function(client, display, ...) { + # IRC has no display name distinct from the nick, so this is a NICK + # change. The server can refuse it (collision, length, restricted + # characters) and says so asynchronously in a numeric this adapter + # does not parse, so the local nick is updated optimistically and + # chat_whoami() can be wrong until the next reconnect. + irc_write(client$env$con, sprintf("NICK %s", display)) + client$env$nick <- display + invisible(TRUE) +} diff --git a/R/loopback.R b/R/loopback.R index 987f9ef..122bc88 100644 --- a/R/loopback.R +++ b/R/loopback.R @@ -60,6 +60,8 @@ chat_capabilities.chat_loopback <- function(client, ...) { list(threads = TRUE, thread_replies = TRUE, edits = FALSE, reactions = FALSE, reaction_events = FALSE, channel_info = FALSE, members = FALSE, invites = FALSE, join = FALSE, whoami = TRUE, + channels = TRUE, history = TRUE, pending = FALSE, + mark_read = FALSE, set_identity = FALSE, relogin = FALSE, files = FALSE, typing = FALSE, e2ee = FALSE, identity_override = TRUE, markup_dialects = c("plain", "markdown"), max_message_bytes = NA_integer_) @@ -72,3 +74,33 @@ chat_whoami.chat_loopback <- function(client, ...) { # something stable to compare against across a test. chat_identity("loopback") } + +#' @export +chat_channels.chat_loopback <- function(client, ...) { + unique(vapply(client$env$log, function(m) m$channel, character(1))) +} + +#' @export +chat_history.chat_loopback <- function(client, channel, limit = 50L, + before = NULL, ...) { + log <- client$env$log + keep <- vapply(log, function(m) identical(m$channel, channel), logical(1)) + log <- log[keep] + if (!is.null(before)) { + pos <- which(vapply(log, function(m) identical(m$id, before), + logical(1))) + if (length(pos)) { + if (pos[[1L]] > 1L) { + log <- log[seq_len(pos[[1L]] - 1L)] + } else { + log <- list() + } + } + } + # The tail, still oldest-first. limit trims the far end, not the near + # one: "the last 20 messages" means the 20 most recent. + if (length(log) > limit) { + log <- log[seq.int(length(log) - limit + 1L, length(log))] + } + log +} diff --git a/R/matrix-config.R b/R/matrix-config.R new file mode 100644 index 0000000..76cf540 --- /dev/null +++ b/R/matrix-config.R @@ -0,0 +1,228 @@ +#' @title Matrix credential lifecycle +#' @description Where a Matrix client's credentials live, how they are +#' read and written, and how they are refreshed. Split from +#' \code{R/matrix.R} because it is a different concern: that file is +#' about exchanging messages with a homeserver, this one is about +#' having an account to do it with. + +#' Load a Matrix configuration +#' +#' Reads the credentials an application saved, and returns them as a +#' \code{chat_config}: a list carrying the transport's own fields plus +#' whatever else the application stored alongside them, with the app +#' namespace and file path attached as attributes so +#' \code{\link{chat_config_save}} can write it back where it came from. +#' +#' @section Why the extra fields survive: +#' Applications keep their own settings in the same file -- which +#' accounts count as bots, who may open a private conversation, a +#' preferred model. Those are the application's, not the transport's, +#' and a loader that dropped them would make the file unreadable by its +#' owner. They pass through untouched and unvalidated. +#' +#' @param app Application namespace, e.g. \code{"corteza"}. Decides the +#' default path under \code{tools::R_user_dir(app, "config")}. +#' @param path Explicit file path, overriding \code{app}'s default. +#' @param env_var Name of an environment variable that, when set, +#' overrides both. +#' @return A list with class \code{chat_config}. +#' @examples +#' \dontrun{ +#' cfg <- chat_matrix_config(app = "corteza", +#' env_var = "CORTEZA_MATRIX_CONFIG") +#' client <- chat_matrix(mx = cfg) +#' } +#' @export +chat_matrix_config <- function(app = NULL, path = NULL, env_var = NULL) { + matrix_require_client("chat_matrix_config") + args <- list(app = app) + # Passed only when supplied. mx_client_load() distinguishes an + # absent argument from a NULL one for path and env_var: an explicit + # NULL path suppresses the app default rather than falling back to + # it, which would look for the config in the working directory. + if (!is.null(path)) { + args$path <- path + } + if (!is.null(env_var)) { + args$env_var <- env_var + } + cfg <- do.call(mx.client::mx_client_load, args) + chat_config(cfg, app = app, path = attr(cfg, "path") %||% path) +} + +#' Construct a chat_config +#' +#' @param x A named list of configuration fields. +#' @param app Application namespace the config belongs to, or NULL. +#' @param path File the config was read from, or NULL for one that has +#' never been written. +#' @return A list with class \code{chat_config}. +#' @examples +#' chat_config(list(server = "https://ex.invalid", user = "bot"), +#' app = "demo") +#' @export +chat_config <- function(x, app = NULL, path = NULL) { + x <- unclass(x) + stopifnot(is.list(x)) + # app and path travel as attributes rather than as fields, because a + # field would collide with an application that already keeps one by + # that name -- and be written back into the file as though it were + # part of the credentials. + attr(x, "app") <- app + attr(x, "path") <- path + structure(x, class = c("chat_config", "list")) +} + +#' @export +print.chat_config <- function(x, ...) { + # Never the values. A config holds an access token and often a + # password, and printing one at a prompt is how it reaches a + # terminal scrollback, a screenshot, or a pasted bug report. + cat(sprintf(" %s\n", attr(x, "path") %||% "(unsaved)")) + cat(sprintf(" fields: %s\n", paste(sort(names(x)), collapse = ", "))) + invisible(x) +} + +#' Where a Matrix configuration lives +#' +#' @param app Application namespace. +#' @param env_var Name of an environment variable that overrides the +#' default path, or NULL. +#' @param legacy Return the pre-\code{R_user_dir} location instead, for +#' an application that needs to migrate an older file. +#' @return The file path (character). +#' @examples +#' chat_matrix_config_path("demo") +#' @export +chat_matrix_config_path <- function(app, env_var = NULL, legacy = FALSE) { + matrix_require_client("chat_matrix_config_path") + if (isTRUE(legacy)) { + return(mx.client::mx_client_legacy_config_path(app)) + } + if (is.null(env_var)) { + mx.client::mx_client_config_path(app) + } else { + mx.client::mx_client_config_path(app, env_var = env_var) + } +} + +#' Persist a configuration +#' +#' Writes to the file the config came from, at mode 0600. +#' +#' @param config A \code{\link{chat_config}}, or a plain list together +#' with \code{app}/\code{path}. +#' @param app Override the app namespace the config was loaded under. +#' @param path Override the file to write. +#' @return The config, invisibly. +#' @examples +#' \dontrun{ +#' cfg$operators <- "@troy:example.org" +#' chat_config_save(cfg) +#' } +#' @export +chat_config_save <- function(config, app = NULL, path = NULL) { + matrix_require_client("chat_config_save") + app <- app %||% attr(config, "app") + path <- path %||% attr(config, "path") + if (is.null(app) && is.null(path)) { + stop("chat_config_save(): nowhere to write. The config carries ", + "neither an app nor a path, so pass one.", call. = FALSE) + } + args <- list(unclass_config(config)) + if (!is.null(app)) { + args$app <- app + } + if (!is.null(path)) { + args$path <- path + } + do.call(mx.client::mx_client_save, args) + invisible(config) +} + +# The plain list underneath, with the class and the bookkeeping +# attributes stripped. mx.client writes whatever it is handed, so an +# attribute left on here becomes a field in the file. +unclass_config <- function(config) { + x <- unclass(config) + attr(x, "app") <- NULL + attr(x, "path") <- NULL + x +} + +#' Configure a Matrix account interactively +#' +#' Logs in to a homeserver, resolves the room, and writes the resulting +#' credentials. +#' +#' @param server Homeserver base URL. +#' @param user Localpart or full user id. +#' @param password Account password. +#' @param room Room id or alias to record as the default. +#' @param app Application namespace to save under. +#' @param path Explicit path to save to, overriding \code{app}. +#' @param device_id Device id to log in as. Reusing one keeps an +#' existing E2EE identity; a new one starts a new device. +#' @param extra Named list of application fields to store alongside the +#' credentials. +#' @return A \code{\link{chat_config}}, invisibly. +#' @examples +#' \dontrun{ +#' cfg <- chat_matrix_configure(server = "https://matrix.example.org", +#' user = "bot", password = pw, +#' room = "#lab:example.org", app = "corteza") +#' } +#' @export +chat_matrix_configure <- function(server, user, password, room = NULL, + app = NULL, path = NULL, device_id = NULL, + extra = list()) { + matrix_require_client("chat_matrix_configure") + cfg <- mx.client::mx_client_configure(server = server, user = user, + password = password, room = room, app = app, path = path, + device_id = device_id, extra = extra) + invisible(chat_config(cfg, app = app, path = attr(cfg, "path") %||% path)) +} + +#' @export +chat_relogin.chat_matrix <- function(client, ...) { + refreshed <- mx.client::mx_client_relogin(client$env$mx) + # Into the client, not back to the caller. A relogin that handed the + # new credentials out and left the old ones in place would leave two + # copies, and whichever the next call happened to use decides + # whether it works. + client$env$mx <- refreshed + invisible(TRUE) +} + +#' @export +chat_set_identity.chat_matrix <- function(client, display, ...) { + fn <- client$identity_fn %||% mx.client::mx_set_displayname + fn(client$env$mx, display) + # mx_set_displayname() wraps itself in mx_with_relogin(), which + # persists a refreshed config before retrying and then returns only + # TRUE -- discarding the client it refreshed. So the live token may + # now be on disk and not in memory, and the only way to pick it up + # is to read it back. + # + # Unconditionally, not on success: a relogin whose retry then failed + # (rate limit, transient 5xx) still rotated the token and still + # wrote it. Reloading only on success is how the following send goes + # out on the token the homeserver has already rejected. + path <- attr(client$env$mx, "path") + app <- attr(client$env$mx, "app") %||% client$app + if (!is.null(path) || !is.null(app)) { + args <- list() + if (!is.null(app)) { + args$app <- app + } + if (!is.null(path)) { + args$path <- path + } + fresh <- tryCatch(do.call(mx.client::mx_client_load, args), + error = function(e) NULL) + if (!is.null(fresh)) { + client$env$mx <- fresh + } + } + invisible(TRUE) +} diff --git a/R/matrix.R b/R/matrix.R index 81e9163..0855571 100644 --- a/R/matrix.R +++ b/R/matrix.R @@ -123,6 +123,17 @@ #' \code{mx.api::mx_room_members}. Leave NULL in production. #' @param .join Testing seam: replacement for #' \code{mx.api::mx_room_join}. Leave NULL in production. +#' @param .channels Testing seam: replacement for +#' \code{mx.api::mx_rooms}. Leave NULL in production. +#' @param .history Testing seam: replacement for +#' \code{mx.api::mx_messages}. Leave NULL in production. +#' @param .pending Testing seam: replacement for \code{mx.api::mx_sync}, +#' used by \code{\link{chat_pending}} for its cursorless snapshot. +#' Leave NULL in production. +#' @param .read Testing seam: replacement for +#' \code{mx.api::mx_read_receipt}. Leave NULL in production. +#' @param .identity Testing seam: replacement for +#' \code{mx.client::mx_set_displayname}. Leave NULL in production. #' @return A \code{chat_client} of class \code{chat_matrix}. #' \code{\link{chat_poll}} on this class returns \code{first_run} and #' \code{client} alongside \code{messages}, \code{cursor}, and @@ -138,7 +149,9 @@ chat_matrix <- function(app = NULL, path = NULL, save_cursor = TRUE, crypto_store = NULL, .sync = NULL, .extract = NULL, .send = NULL, .media = NULL, .typing = NULL, .crypto = NULL, .save = NULL, .react = NULL, - .info = NULL, .members = NULL, .join = NULL) { + .info = NULL, .members = NULL, .join = NULL, + .channels = NULL, .history = NULL, .pending = NULL, + .read = NULL, .identity = NULL) { seams <- list(.sync, .extract, .send, .media) if ((is.null(mx) || any(vapply(seams, is.null, logical(1)))) && !requireNamespace("mx.client", quietly = TRUE)) { @@ -192,6 +205,9 @@ chat_matrix <- function(app = NULL, path = NULL, save_cursor = TRUE, save_fn = .save, typing_fn = .typing, react_fn = .react, info_fn = .info, members_fn = .members, join_fn = .join, + channels_fn = .channels, history_fn = .history, + pending_fn = .pending, read_fn = .read, + identity_fn = .identity, crypto_ops = matrix_crypto_ops(.crypto)), class = c("chat_matrix", "chat_client")) } @@ -319,6 +335,18 @@ matrix_kind <- function(msgtype) { "message") } +# The same mapping, but NA rather than "message" for a msgtype the +# contract has no word for. chat_poll() can afford the lenient version: +# its extractor has already filtered to the msgtypes it was asked for. +# chat_history() reads the timeline raw, so an m.image would otherwise +# arrive as a "message" whose body is a filename. +matrix_kind_strict <- function(msgtype) { + if (!is.character(msgtype) || length(msgtype) != 1L || is.na(msgtype)) { + return(NA_character_) + } + unname(c(m.text = "message", m.notice = "notice", m.emote = "emote")[msgtype]) +} + # chat_poll's `...` reaches mx_sync_update, so a caller can pass a # server-side `filter` or an explicit `path`. chat_send's reaches # mx_send_text, whose `mentions` argument is the only way to emit an @@ -468,8 +496,15 @@ chat_poll.chat_matrix <- function(client, since = NULL, timeout = NULL, ...) { # acts on either has the same backfill problem it has with messages # and solves it the same way -- what it must not have to learn is # that one of the three lists plays by a different rule. - list(messages = messages, cursor = res$client$sync_token, - first_run = isTRUE(res$first_run), client = res$client, + # No `client`. The post-sync config used to be handed back so a + # consumer could keep driving mx.api with a token this poll may have + # rotated. That made the consumer the owner of a credential the + # adapter had already refreshed in place, and the two stayed in step + # only because both wrote the same file. Everything that needed it + # is a verb now, and the refreshed config is on `client$env$mx` + # where the next call will find it. + list(messages = messages, cursor = client$env$mx$sync_token, + first_run = isTRUE(res$first_run), reactions = matrix_reactions(client, res$sync, event_pos), invites = matrix_invites(client, res$sync), raw = res$sync) @@ -570,8 +605,8 @@ chat_react.chat_matrix <- function(client, channel, message_id, key, ...) { # indicator costs nothing; a dropped reaction is an acknowledgement # the sender believes it made and no one can see. react_fn <- client$react_fn %||% mx.api::mx_react - invisible(react_fn(mx.client::mx_client_session(client$env$mx), channel, - message_id, key)) + sess <- mx.client::mx_client_session(client$env$mx) + invisible(react_fn(sess, channel, message_id, key)) } #' @export @@ -600,18 +635,18 @@ chat_join.chat_matrix <- function(client, channel, ...) { # Errors propagate. A join that quietly failed leaves the caller # believing it is in a room it will never hear a word from, which is # indistinguishable from an idle room. + sess <- mx.client::mx_client_session(client$env$mx) join_fn <- client$join_fn %||% mx.api::mx_room_join - invisible(as.character( - join_fn(mx.client::mx_client_session(client$env$mx), channel))) + invisible(as.character(join_fn(sess, channel))) } #' @export chat_members.chat_matrix <- function(client, channel, ...) { # Errors propagate. An empty room and an unanswerable question are # different things, and character() has to mean only the first. + sess <- mx.client::mx_client_session(client$env$mx) members_fn <- client$members_fn %||% mx.api::mx_room_members - as.character(members_fn(mx.client::mx_client_session(client$env$mx), - channel)) + as.character(members_fn(sess, channel)) } #' @export @@ -664,9 +699,11 @@ chat_capabilities.chat_matrix <- function(client, ...) { reactions = TRUE, reaction_events = matrix_reactions_available(), channel_info = TRUE, members = TRUE, invites = matrix_invites_available(), join = TRUE, whoami = TRUE, - files = !isTRUE(client$e2ee), typing = TRUE, - e2ee = isTRUE(client$e2ee), identity_override = FALSE, - markup_dialects = c("plain", "markdown"), + channels = TRUE, history = TRUE, + pending = matrix_invites_available(), mark_read = TRUE, + set_identity = TRUE, relogin = TRUE, files = !isTRUE(client$e2ee), + typing = TRUE, e2ee = isTRUE(client$e2ee), + identity_override = FALSE, markup_dialects = c("plain", "markdown"), max_message_bytes = NA_integer_) } @@ -736,3 +773,103 @@ chat_addressed.chat_matrix <- function(client, message, ...) { a } } + +# mx.client is a Suggests, and the config lifecycle has no seams: unlike +# poll and send, there is nothing to fake -- these functions exist to +# touch a real file and a real homeserver. +matrix_require_client <- function(what) { + if (!requireNamespace("mx.client", quietly = TRUE)) { + stop(what, "() requires the 'mx.client' package. Install it first.", + call. = FALSE) + } + invisible(TRUE) +} + +#' @export +chat_channels.chat_matrix <- function(client, ...) { + # Built before the call, not inside it. R would otherwise hand the + # seam a promise, and a seam that ignores its session argument -- + # every test double here does -- never forces it. That makes a + # config too broken to build a session from look fine under test and + # fail only in production, where the real mx.api function reads it. + sess <- mx.client::mx_client_session(client$env$mx) + fn <- client$channels_fn %||% mx.api::mx_rooms + as.character(fn(sess)) +} + +#' @export +chat_history.chat_matrix <- function(client, channel, limit = 50L, + before = NULL, ...) { + fn <- client$history_fn %||% mx.api::mx_messages + sess <- mx.client::mx_client_session(client$env$mx) + args <- list(sess, channel, dir = "b", limit = as.integer(limit)) + if (!is.null(before)) { + args$from <- before + } + res <- do.call(fn, args) + chunk <- res$chunk %||% list() + # Matrix pages backwards, so the chunk arrives newest-first. The + # contract promises oldest-first, and this is the one place that + # knows which direction it asked for. + chunk <- rev(chunk) + out <- list() + for (ev in chunk) { + if (!isTRUE(ev$type == "m.room.message")) { + next + } + msgtype <- ev$content$msgtype %||% "" + kind <- matrix_kind_strict(msgtype) + if (is.na(kind)) { + # A msgtype the contract has no word for -- an image, a + # file. Skipping beats inventing a kind: a consumer + # replaying history into a transcript would otherwise get an + # empty "message" where a picture was. + next + } + ms <- ev$origin_server_ts + out[[length(out) + 1L]] <- chat_message( + id = as.character(ev$event_id), + channel = as.character(channel), + sender = as.character(ev$sender %||% ""), + body = as.character(ev$content$body %||% ""), + ts = if (is.null(ms)) as.POSIXct(NA) else + as.POSIXct(ms / 1000, origin = "1970-01-01"), + markup = "plain", kind = kind, + self = identical(ev$sender, client$env$mx$user_id), + mentions = unlist(ev$content[["m.mentions"]]$user_ids, + use.names = FALSE), + raw = ev) + } + out +} + +#' @export +chat_pending.chat_matrix <- function(client, ...) { + if (!matrix_invites_available()) { + stop("chat_pending() needs an mx.client with ", + "mx_extract_invite_records(). Upgrade mx.client.", call. = FALSE) + } + fn <- client$pending_fn %||% mx.api::mx_sync + # No `since`. That is the whole point: a homeserver that only + # reports invites newer than the cursor will never mention, in the + # poll loop, an invitation issued while this client was down. + # timeout 0 makes it a snapshot rather than a long poll. + sess <- mx.client::mx_client_session(client$env$mx) + sync <- fn(sess, timeout = 0L) + recs <- mx.client::mx_extract_invite_records(sync, client$env$mx$user_id) + list(invites = lapply(recs, function(r) { + chat_invite(channel = as.character(r$room_id), + inviter = r$inviter %||% NA_character_, raw = r) + })) +} + +#' @export +chat_mark_read.chat_matrix <- function(client, channel, message_id, ...) { + ok <- tryCatch({ + fn <- client$read_fn %||% mx.api::mx_read_receipt + sess <- mx.client::mx_client_session(client$env$mx) + fn(sess, channel, message_id) + TRUE + }, error = function(e) FALSE) + invisible(ok) +} diff --git a/R/slack.R b/R/slack.R index c6ff06b..6941fde 100644 --- a/R/slack.R +++ b/R/slack.R @@ -384,6 +384,8 @@ chat_capabilities.chat_slack <- function(client, ...) { # invitation to hand a consumer. Joining an open channel is a # different thing, and does work. invites = FALSE, join = TRUE, whoami = TRUE, + channels = TRUE, history = TRUE, pending = FALSE, + mark_read = TRUE, set_identity = TRUE, relogin = FALSE, files = FALSE, typing = FALSE, e2ee = FALSE, identity_override = TRUE, markup_dialects = c("plain", "markdown"), max_message_bytes = 40000L) @@ -428,3 +430,97 @@ chat_addressed.chat_slack <- function(client, message, ...) { # not mention it, and Slack agrees -- no notification goes out. nzchar(body) && grepl(sprintf("<@%s>", id), body, fixed = TRUE) } + +#' @export +chat_channels.chat_slack <- function(client, ...) { + api <- client$api_fn %||% slackr::call_slack_api + out <- character() + cursor <- "" + repeat { + args <- list("/api/conversations.list", .method = "GET", + token = client$token, limit = 200L, + types = "public_channel,private_channel") + if (nzchar(cursor)) { + args$cursor <- cursor + } + body <- slack_body(do.call(api, args)) + slack_stop_for_error(body, "conversations.list") + for (ch in body$channels %||% list()) { + # Only the ones this bot is in. conversations.list reports + # every channel the workspace has, and a consumer reading + # this as "where I can post" would fan out across the org. + if (isTRUE(ch$is_member)) { + out <- c(out, as.character(ch$id)) + } + } + cursor <- body$response_metadata$next_cursor %||% "" + if (!nzchar(cursor)) { + break + } + } + out +} + +#' @export +chat_history.chat_slack <- function(client, channel, limit = 50L, + before = NULL, ...) { + api <- client$api_fn %||% slackr::call_slack_api + args <- list("/api/conversations.history", .method = "GET", + token = client$token, channel = sub("^#", "", channel), + limit = as.integer(limit)) + if (!is.null(before)) { + args$latest <- before + args$inclusive <- FALSE + } + body <- slack_body(do.call(api, args)) + slack_stop_for_error(body, "conversations.history") + msgs <- body$messages %||% list() + # Slack pages backwards too, newest first. The contract promises the + # other order. + msgs <- rev(msgs) + out <- list() + for (m in msgs) { + # Joins, leaves, channel renames. They carry a subtype and are + # not conversation. + if (!is.null(m$subtype)) { + next + } + ts <- suppressWarnings(as.numeric(m$ts)) + out[[length(out) + 1L]] <- chat_message( + id = as.character(m$ts), + channel = as.character(channel), + sender = as.character(m$user %||% m$bot_id %||% ""), + body = as.character(m$text %||% ""), + ts = if (is.na(ts)) as.POSIXct(NA) else + as.POSIXct(ts, origin = "1970-01-01"), + markup = "plain", kind = "message", + thread = m$thread_ts, raw = m) + } + out +} + +#' @export +chat_mark_read.chat_slack <- function(client, channel, message_id, ...) { + ok <- tryCatch({ + api <- client$api_fn %||% slackr::call_slack_api + resp <- api("/api/conversations.mark", .method = "POST", + token = client$token, + body = list(channel = sub("^#", "", channel), ts = message_id)) + slack_stop_for_error(resp, "conversations.mark") + TRUE + }, error = function(e) FALSE) + invisible(ok) +} + +#' @export +chat_set_identity.chat_slack <- function(client, display, ...) { + api <- client$api_fn %||% slackr::call_slack_api + resp <- api("/api/users.profile.set", .method = "POST", + token = client$token, + body = list(profile = sprintf('{"display_name":"%s"}', + gsub('"', '\\\\"', display)))) + slack_stop_for_error(resp, "users.profile.set") + # The cached identity carries a display name, and it is now wrong. + client$env$whoami <- NULL + invisible(TRUE) +} diff --git a/inst/tinytest/test_matrix.R b/inst/tinytest/test_matrix.R index 95e3266..c0344bd 100644 --- a/inst/tinytest/test_matrix.R +++ b/inst/tinytest/test_matrix.R @@ -7,9 +7,17 @@ # device_id is here because an Olm account belongs to a device: an e2ee # client refuses a config that cannot name one. +# +# token, because mx_client_session() refuses a config without one and +# every mx.api-backed method builds a session before calling out. It was +# missing here for a long time and nothing noticed: R hands the session +# to the seam as a promise, and no seam double reads its session +# argument, so mx_client_session() never actually ran. The methods force +# it now, which is what production does the instant the real mx.api +# function touches it. fake_mx <- function(sync_token = NULL, user_id = "@bot:ex", device_id = "DEV1") { - list(user_id = user_id, server = "https://ex.invalid", + list(user_id = user_id, server = "https://ex.invalid", token = "tok", device_id = device_id, sync_token = sync_token) } @@ -172,14 +180,22 @@ trace5$extract <- list() chat_poll(seam_client(record = trace5, save_cursor = FALSE)) expect_false(trace5$sync[[1L]]$save) -# ---- Poll: the post-sync client comes back out ---- -# A relogin can swap the token mid-poll. A consumer that keeps driving -# mx.api off its own pre-poll copy spends the rest of the cycle -# authenticating with the token the homeserver just rejected. - -expect_true("client" %in% names(got)) -expect_identical(got$client$sync_token, "s1") -expect_identical(got$client$user_id, "@bot:ex") +# ---- Poll: the post-sync client stays inside ---- +# It used to come back out. A relogin can swap the token mid-poll, and +# handing the refreshed config to the caller made the caller responsible +# for a credential the adapter had already replaced in place -- two +# copies, in step only because both wrote the same file. +expect_false("client" %in% names(got)) +# The cursor still reports the post-sync token, which is the one thing a +# consumer legitimately wanted off that config. +expect_identical(got$cursor, "s1") +# And the refreshed credentials are live on the client, so the next call +# through the contract uses them without anyone passing them along. +local({ + cl <- seam_client(token = "rotated") + chat_poll(cl) + expect_identical(cl$env$mx$sync_token, "rotated") +}) # ---- Poll: timestamps ---- # The one field that is silently wrong rather than loudly missing when @@ -1553,3 +1569,178 @@ local({ }) expect_true(chat_capabilities(seam_client())$whoami) + +# ---- State: channels ---- +local({ + cl <- seam_client(.channels = function(session) c("!a:ex", "!b:ex")) + expect_identical(chat_channels(cl), c("!a:ex", "!b:ex")) +}) +expect_error(chat_channels(structure(list(), class = c("chat_nothing", + "chat_client"))), + "not supported by this adapter") + +# ---- State: history ---- +hev <- function(id, body = "hi", msgtype = "m.text", ts = 1700000000000, + sender = "@ann:ex", mentions = NULL) { + content <- list(msgtype = msgtype, body = body) + if (!is.null(mentions)) { + content[["m.mentions"]] <- list(user_ids = as.list(mentions)) + } + list(type = "m.room.message", event_id = id, sender = sender, + origin_server_ts = ts, content = content) +} + +local({ + seen <- NULL + cl <- seam_client(.history = function(session, room_id, ...) { + seen <<- c(list(room_id = room_id), list(...)) + # Newest first, the way Matrix pages backwards. + list(chunk = list(hev("$3", "third", ts = 3000), + hev("$2", "second", ts = 2000), + hev("$1", "first", ts = 1000))) + }) + h <- chat_history(cl, "!a:ex", limit = 3L) + expect_identical(seen$room_id, "!a:ex") + expect_identical(seen$dir, "b") + expect_identical(seen$limit, 3L) + # Oldest first, whatever direction the platform paged in. A consumer + # replaying this into a transcript gets a conversation, not its + # reverse -- and a reversed transcript reads plausibly enough that + # nothing errors. + expect_identical(vapply(h, function(m) m$id, character(1)), + c("$1", "$2", "$3")) + expect_identical(h[[1L]]$body, "first") + expect_identical(h[[1L]]$channel, "!a:ex") + expect_true(all(diff(vapply(h, function(m) as.numeric(m$ts), + numeric(1))) > 0)) +}) + +local({ + # `before` pages backwards from a known id, and only then. + seen <- NULL + cl <- seam_client(.history = function(session, room_id, ...) { + seen <<- list(...) + list(chunk = list()) + }) + chat_history(cl, "!a:ex") + expect_false("from" %in% names(seen)) + chat_history(cl, "!a:ex", before = "$9") + expect_identical(seen$from, "$9") +}) + +local({ + # A msgtype the contract has no word for is dropped, not renamed. + # matrix_kind() answers "message" for anything, so an m.image would + # otherwise arrive as a text message whose body is a filename. + cl <- seam_client(.history = function(...) { + list(chunk = list(hev("$2", "cat.png", msgtype = "m.image"), + hev("$1", "look"))) + }) + h <- chat_history(cl, "!a:ex") + expect_identical(length(h), 1L) + expect_identical(h[[1L]]$id, "$1") +}) +expect_identical(chat.api:::matrix_kind_strict("m.image"), NA_character_) +expect_identical(chat.api:::matrix_kind_strict("m.notice"), "notice") +expect_identical(chat.api:::matrix_kind_strict(NULL), NA_character_) + +local({ + # Non-message state events (joins, topic changes) are not history. + cl <- seam_client(.history = function(...) { + list(chunk = list(list(type = "m.room.member", event_id = "$m"), + hev("$1", "look"))) + }) + expect_identical(length(chat_history(cl, "!a:ex")), 1L) +}) + +local({ + # self and mentions survive the trip, so a consumer can tell its own + # backfilled traffic from everyone else's. + cl <- seam_client(.history = function(...) { + list(chunk = list(hev("$1", "mine", sender = "@bot:ex", + mentions = "@ann:ex"))) + }) + h <- chat_history(cl, "!a:ex") + expect_true(h[[1L]]$self) + expect_identical(h[[1L]]$mentions, "@ann:ex") +}) + +# ---- State: pending ---- +local({ + seen <- NULL + cl <- seam_client(.pending = function(session, timeout = NULL, ...) { + seen <<- list(timeout = timeout, args = list(...)) + list(rooms = list(invite = list(`!a:ex` = list(invite_state = list( + events = list(list(type = "m.room.member", + sender = "@ann:ex", + state_key = "@bot:ex", + content = list(membership = "invite")))))))) + }) + p <- chat_pending(cl) + # timeout 0: a snapshot, not a long poll. + expect_identical(seen$timeout, 0L) + # And no `since`. That is the whole point -- a homeserver that only + # reports invites newer than the cursor never mentions, in the poll + # loop, one issued while this client was down. + expect_false("since" %in% names(seen$args)) + expect_identical(length(p$invites), 1L) + expect_inherits(p$invites[[1L]], "chat_invite") + expect_identical(p$invites[[1L]]$channel, "!a:ex") + expect_identical(p$invites[[1L]]$inviter, "@ann:ex") +}) + +local({ + # Nothing pending is an empty list, not NULL: a consumer looping + # over it should not have to test for both. + cl <- seam_client(.pending = function(...) list(rooms = list(invite = list()))) + expect_identical(chat_pending(cl)$invites, list()) +}) + +# ---- State: mark read ---- +local({ + seen <- NULL + cl <- seam_client(.read = function(session, room_id, event_id, ...) { + seen <<- list(room_id = room_id, event_id = event_id) + TRUE + }) + expect_true(chat_mark_read(cl, "!a:ex", "$1")) + expect_identical(seen$room_id, "!a:ex") + expect_identical(seen$event_id, "$1") +}) + +# A failed receipt is FALSE, not a throw. Unlike a reaction, nobody is +# waiting on a read marker -- it costs a human a little context about +# what the bot has seen, and nothing more. +expect_false(chat_mark_read(seam_client(.read = function(...) stop("boom")), + "!a:ex", "$1")) +# An adapter without one says nothing rather than failing, for the same +# reason. +expect_false(chat_mark_read(structure(list(), + class = c("chat_nothing", "chat_client")), + "!a:ex", "$1")) + +# ---- Credentials: set identity ---- +local({ + seen <- NULL + cl <- seam_client(.identity = function(client, name, ...) { + seen <<- list(name = name) + TRUE + }) + expect_true(chat_set_identity(cl, "corteza [opus]")) + expect_identical(seen$name, "corteza [opus]") +}) + +expect_error(chat_set_identity(structure(list(), + class = c("chat_nothing", + "chat_client")), "x"), + "not supported by this adapter") + +local({ + caps <- chat_capabilities(seam_client()) + expect_true(caps$channels) + expect_true(caps$history) + expect_true(caps$mark_read) + expect_true(caps$set_identity) + expect_true(caps$relogin) + expect_identical(caps$pending, chat.api:::matrix_invites_available()) +}) diff --git a/inst/tinytest/test_matrix_mxclient.R b/inst/tinytest/test_matrix_mxclient.R index be82b82..4a94f78 100644 --- a/inst/tinytest/test_matrix_mxclient.R +++ b/inst/tinytest/test_matrix_mxclient.R @@ -822,3 +822,115 @@ expect_identical(names(formals(mx.api::mx_room_topic))[1:2], c("session", "room_id")) expect_identical(names(formals(mx.api::mx_room_members))[1:2], c("session", "room_id")) + +# ---- Config lifecycle: drift against mx.client's real signatures ---- +# These have no seams: they exist to touch a real file, so there is +# nothing to fake. Drift detection is the only guard available. +for (fn in c("mx_client_load", "mx_client_save", "mx_client_config_path", + "mx_client_legacy_config_path", "mx_client_configure", + "mx_client_relogin", "mx_set_displayname")) { + expect_true(is.function(getExportedValue("mx.client", fn)), info = fn) +} +expect_true(all(c("app", "path", "env_var") %in% + names(formals(mx.client::mx_client_load)))) +expect_true(all(c("client", "app", "path") %in% + names(formals(mx.client::mx_client_save)))) +expect_identical(names(formals(mx.client::mx_client_save))[1L], "client") +expect_true(all(c("server", "user", "password", "room", "app", "path", + "device_id", "extra") %in% + names(formals(mx.client::mx_client_configure)))) + +# ---- chat_config ---- +local({ + cfg <- chat_config(list(server = "https://ex.invalid", user = "bot", + operators = "@troy:ex"), + app = "demo", path = "/tmp/demo.json") + expect_inherits(cfg, "chat_config") + # An application's own fields pass through. corteza keeps `bots`, + # `operators` and a model preference in the same file, and a loader + # that dropped them would make the file unreadable by its owner. + expect_identical(cfg$operators, "@troy:ex") + expect_identical(attr(cfg, "app"), "demo") + expect_identical(attr(cfg, "path"), "/tmp/demo.json") + # app and path are attributes, never fields: a field would collide + # with an application that already keeps one by that name, and would + # be written into the file as though it were part of the config. + expect_false("app" %in% names(cfg)) + expect_false("path" %in% names(cfg)) + expect_false("app" %in% names(chat.api:::unclass_config(cfg))) + expect_null(attr(chat.api:::unclass_config(cfg), "path")) +}) + +# print() shows the field names and never the values. A config holds an +# access token and usually a password, and printing one at a prompt is +# how it reaches a scrollback, a screenshot, or a pasted bug report. +local({ + cfg <- chat_config(list(token = "syt_secret", password = "hunter2"), + path = "/tmp/x.json") + out <- paste(capture.output(print(cfg)), collapse = " ") + expect_false(grepl("syt_secret", out, fixed = TRUE)) + expect_false(grepl("hunter2", out, fixed = TRUE)) + expect_true(grepl("token", out, fixed = TRUE)) + expect_true(grepl("/tmp/x.json", out, fixed = TRUE)) +}) + +# ---- Round trip through a real file ---- +local({ + dir <- tempfile("cfg-") + dir.create(dir) + on.exit(unlink(dir, recursive = TRUE), add = TRUE) + path <- file.path(dir, "matrix.json") + + cfg <- chat_config(list(server = "https://ex.invalid", user = "bot", + token = "tok", user_id = "@bot:ex", + device_id = "DEV1", operators = "@troy:ex"), + path = path) + chat_config_save(cfg) + expect_true(file.exists(path)) + # 0600. The file holds an access token. + expect_identical(substr(as.character(file.mode(path)), 1L, 3L), "600") + + back <- chat_matrix_config(path = path) + expect_inherits(back, "chat_config") + expect_identical(back$user_id, "@bot:ex") + expect_identical(back$operators, "@troy:ex") + # It knows where it came from, so chat_config_save() can write it + # back without being told twice. + expect_identical(normalizePath(attr(back, "path")), normalizePath(path)) + + back$operators <- c("@troy:ex", "@jorge:ex") + chat_config_save(back) + expect_identical(chat_matrix_config(path = path)$operators, + c("@troy:ex", "@jorge:ex")) +}) + +# A config with nowhere to go says so, rather than writing somewhere +# arbitrary and reporting success. +expect_error(chat_config_save(chat_config(list(user = "bot"))), + "nowhere to write") + +# ---- Paths ---- +expect_true(is.character(chat_matrix_config_path("demo"))) +expect_true(nzchar(chat_matrix_config_path("demo"))) +expect_false(identical(chat_matrix_config_path("demo"), + chat_matrix_config_path("demo", legacy = TRUE))) +local({ + var <- "CHAT_API_TEST_CONFIG" + old <- Sys.getenv(var, unset = NA) + on.exit(if (is.na(old)) Sys.unsetenv(var) else + do.call(Sys.setenv, stats::setNames(list(old), var)), add = TRUE) + do.call(Sys.setenv, stats::setNames(list("/tmp/override.json"), var)) + expect_identical(chat_matrix_config_path("demo", env_var = var), + "/tmp/override.json") +}) + +# ---- A config drives a client ---- +local({ + cfg <- chat_config(list(server = "https://ex.invalid", user = "bot", + token = "tok", user_id = "@bot:ex", + device_id = "DEV1")) + cl <- chat_matrix(mx = cfg, .sync = function(...) NULL, + .extract = function(...) list(), + .send = function(...) "$1", .media = function(...) NULL) + expect_identical(chat_whoami(cl)$id, "@bot:ex") +}) diff --git a/man/chat_channels.Rd b/man/chat_channels.Rd new file mode 100644 index 0000000..33b4383 --- /dev/null +++ b/man/chat_channels.Rd @@ -0,0 +1,24 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{chat_channels} +\alias{chat_channels} +\title{List the channels this client is in} +\usage{ +chat_channels(client, ...) +} +\arguments{ +\item{client}{A \code{chat_client}.} + +\item{...}{Adapter-specific options.} +} +\value{ +Character vector of channel identifiers. +} +\description{ +The state half of the contract. \code{\link{chat_poll}} answers "what +changed since my cursor"; this and its siblings answer "what is true +now", which is the question a process asks when it starts up with no +useful cursor at all. +} +\examples{ +chat_channels(chat_loopback()) +} diff --git a/man/chat_config.Rd b/man/chat_config.Rd new file mode 100644 index 0000000..e8f23d9 --- /dev/null +++ b/man/chat_config.Rd @@ -0,0 +1,25 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{chat_config} +\alias{chat_config} +\title{Construct a chat_config} +\usage{ +chat_config(x, app = NULL, path = NULL) +} +\arguments{ +\item{x}{A named list of configuration fields.} + +\item{app}{Application namespace the config belongs to, or NULL.} + +\item{path}{File the config was read from, or NULL for one that has +never been written.} +} +\value{ +A list with class \code{chat_config}. +} +\description{ +Construct a chat_config +} +\examples{ +chat_config(list(server = "https://ex.invalid", user = "bot"), + app = "demo") +} diff --git a/man/chat_config_save.Rd b/man/chat_config_save.Rd new file mode 100644 index 0000000..a7aca75 --- /dev/null +++ b/man/chat_config_save.Rd @@ -0,0 +1,27 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{chat_config_save} +\alias{chat_config_save} +\title{Persist a configuration} +\usage{ +chat_config_save(config, app = NULL, path = NULL) +} +\arguments{ +\item{config}{A \code{\link{chat_config}}, or a plain list together +with \code{app}/\code{path}.} + +\item{app}{Override the app namespace the config was loaded under.} + +\item{path}{Override the file to write.} +} +\value{ +The config, invisibly. +} +\description{ +Writes to the file the config came from, at mode 0600. +} +\examples{ +\dontrun{ +cfg$operators <- "@troy:example.org" +chat_config_save(cfg) +} +} diff --git a/man/chat_history.Rd b/man/chat_history.Rd new file mode 100644 index 0000000..c5a3c18 --- /dev/null +++ b/man/chat_history.Rd @@ -0,0 +1,47 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{chat_history} +\alias{chat_history} +\title{Read a channel's recent messages} +\usage{ +chat_history(client, channel, limit = 50L, before = NULL, ...) +} +\arguments{ +\item{client}{A \code{chat_client}.} + +\item{channel}{Channel/room identifier.} + +\item{limit}{Maximum messages to return.} + +\item{before}{Return messages older than this message id, for paging +backwards; NULL starts from the most recent.} + +\item{...}{Adapter-specific options.} +} +\value{ +A list of \code{\link{chat_message}}, oldest first. +} +\description{ +Independent of the poll cursor: a restarted process uses this to +recover the context it lost, and asking for it must not move the +cursor or consume anything. +} +\section{Order}{ +Chronological, oldest first, whatever the platform's native direction +is. Matrix \code{dir = "b"} and Slack \code{conversations.history} +both hand back newest-first and every consumer replaying history into +a transcript has to flip it. One flip in the adapter beats one per +consumer, and a consumer that gets it wrong produces a transcript +that reads backwards without erroring. + +} +\section{Overlap with chat_poll}{ +The same message can arrive from both, and adapters must return the +same \code{id} for it either way. That id is the only thing a consumer +has to deduplicate on -- a startup backfill and the first poll after +it routinely cover the same events. +} +\examples{ +cl <- chat_loopback() +chat_send(cl, "general", "hello") +chat_history(cl, "general") +} diff --git a/man/chat_mark_read.Rd b/man/chat_mark_read.Rd new file mode 100644 index 0000000..dd7591d --- /dev/null +++ b/man/chat_mark_read.Rd @@ -0,0 +1,32 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{chat_mark_read} +\alias{chat_mark_read} +\title{Mark a message as read} +\usage{ +chat_mark_read(client, channel, message_id, ...) +} +\arguments{ +\item{client}{A \code{chat_client}.} + +\item{channel}{Channel/room identifier.} + +\item{message_id}{The message to mark read, and everything before it.} + +\item{...}{Adapter-specific options.} +} +\value{ +TRUE if the marker was sent, FALSE otherwise, invisibly. +} +\description{ +The default is a quiet FALSE, on \code{\link{chat_typing}}'s +reasoning rather than \code{\link{chat_react}}'s: a read marker that +does not appear costs a human a little context about what the bot has +seen, and nothing more. Nobody is waiting on it the way they wait on +an acknowledgement. +} +\details{ +Write-only. Reading other participants' read state is a much larger +surface -- per-user, per-device, and absent entirely on some +platforms -- and no consumer needs it yet. + +} diff --git a/man/chat_matrix.Rd b/man/chat_matrix.Rd index 05fcae8..37c4d66 100644 --- a/man/chat_matrix.Rd +++ b/man/chat_matrix.Rd @@ -21,7 +21,12 @@ chat_matrix( .react = NULL, .info = NULL, .members = NULL, - .join = NULL + .join = NULL, + .channels = NULL, + .history = NULL, + .pending = NULL, + .read = NULL, + .identity = NULL ) } \arguments{ @@ -156,6 +161,22 @@ replacing \code{mx.api::mx_room_name} and \item{.join}{Testing seam: replacement for \code{mx.api::mx_room_join}. Leave NULL in production.} + +\item{.channels}{Testing seam: replacement for +\code{mx.api::mx_rooms}. Leave NULL in production.} + +\item{.history}{Testing seam: replacement for +\code{mx.api::mx_messages}. Leave NULL in production.} + +\item{.pending}{Testing seam: replacement for \code{mx.api::mx_sync}, +used by \code{\link{chat_pending}} for its cursorless snapshot. +Leave NULL in production.} + +\item{.read}{Testing seam: replacement for +\code{mx.api::mx_read_receipt}. Leave NULL in production.} + +\item{.identity}{Testing seam: replacement for +\code{mx.client::mx_set_displayname}. Leave NULL in production.} } \value{ A \code{chat_client} of class \code{chat_matrix}. diff --git a/man/chat_matrix_config.Rd b/man/chat_matrix_config.Rd new file mode 100644 index 0000000..8a7eee5 --- /dev/null +++ b/man/chat_matrix_config.Rd @@ -0,0 +1,41 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{chat_matrix_config} +\alias{chat_matrix_config} +\title{Load a Matrix configuration} +\usage{ +chat_matrix_config(app = NULL, path = NULL, env_var = NULL) +} +\arguments{ +\item{app}{Application namespace, e.g. \code{"corteza"}. Decides the +default path under \code{tools::R_user_dir(app, "config")}.} + +\item{path}{Explicit file path, overriding \code{app}'s default.} + +\item{env_var}{Name of an environment variable that, when set, +overrides both.} +} +\value{ +A list with class \code{chat_config}. +} +\description{ +Reads the credentials an application saved, and returns them as a +\code{chat_config}: a list carrying the transport's own fields plus +whatever else the application stored alongside them, with the app +namespace and file path attached as attributes so +\code{\link{chat_config_save}} can write it back where it came from. +} +\section{Why the extra fields survive}{ +Applications keep their own settings in the same file -- which +accounts count as bots, who may open a private conversation, a +preferred model. Those are the application's, not the transport's, +and a loader that dropped them would make the file unreadable by its +owner. They pass through untouched and unvalidated. + +} +\examples{ +\dontrun{ +cfg <- chat_matrix_config(app = "corteza", + env_var = "CORTEZA_MATRIX_CONFIG") +client <- chat_matrix(mx = cfg) +} +} diff --git a/man/chat_matrix_config_path.Rd b/man/chat_matrix_config_path.Rd new file mode 100644 index 0000000..9c81731 --- /dev/null +++ b/man/chat_matrix_config_path.Rd @@ -0,0 +1,25 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{chat_matrix_config_path} +\alias{chat_matrix_config_path} +\title{Where a Matrix configuration lives} +\usage{ +chat_matrix_config_path(app, env_var = NULL, legacy = FALSE) +} +\arguments{ +\item{app}{Application namespace.} + +\item{env_var}{Name of an environment variable that overrides the +default path, or NULL.} + +\item{legacy}{Return the pre-\code{R_user_dir} location instead, for +an application that needs to migrate an older file.} +} +\value{ +The file path (character). +} +\description{ +Where a Matrix configuration lives +} +\examples{ +chat_matrix_config_path("demo") +} diff --git a/man/chat_matrix_configure.Rd b/man/chat_matrix_configure.Rd new file mode 100644 index 0000000..f9f2392 --- /dev/null +++ b/man/chat_matrix_configure.Rd @@ -0,0 +1,49 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{chat_matrix_configure} +\alias{chat_matrix_configure} +\title{Configure a Matrix account interactively} +\usage{ +chat_matrix_configure( + server, + user, + password, + room = NULL, + app = NULL, + path = NULL, + device_id = NULL, + extra = list() +) +} +\arguments{ +\item{server}{Homeserver base URL.} + +\item{user}{Localpart or full user id.} + +\item{password}{Account password.} + +\item{room}{Room id or alias to record as the default.} + +\item{app}{Application namespace to save under.} + +\item{path}{Explicit path to save to, overriding \code{app}.} + +\item{device_id}{Device id to log in as. Reusing one keeps an +existing E2EE identity; a new one starts a new device.} + +\item{extra}{Named list of application fields to store alongside the +credentials.} +} +\value{ +A \code{\link{chat_config}}, invisibly. +} +\description{ +Logs in to a homeserver, resolves the room, and writes the resulting +credentials. +} +\examples{ +\dontrun{ +cfg <- chat_matrix_configure(server = "https://matrix.example.org", + user = "bot", password = pw, + room = "#lab:example.org", app = "corteza") +} +} diff --git a/man/chat_pending.Rd b/man/chat_pending.Rd new file mode 100644 index 0000000..4459fb5 --- /dev/null +++ b/man/chat_pending.Rd @@ -0,0 +1,35 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{chat_pending} +\alias{chat_pending} +\title{Read standing state that is not tied to a cursor} +\usage{ +chat_pending(client, ...) +} +\arguments{ +\item{client}{A \code{chat_client}.} + +\item{...}{Adapter-specific options.} +} +\value{ +A list with \code{invites}, a list of \code{\link{chat_invite}}. +} +\description{ +Today: pending invitations. \code{\link{chat_poll}} reports an +invitation when it arrives, which is no help to a client that was not +running at the time -- and some homeservers only report invites newer +than the \code{since} token, so the poll loop never sees them again. +} +\details{ +This is a separate verb rather than a mode of \code{\link{chat_poll}} +deliberately. Overloading the cursor would make "start from nothing" +and "tell me what is standing" the same call, and a client that asked +for pending invitations and thereby reset its read position would +replay every channel it is in. + +} +\examples{ +\dontrun{ +pending <- chat_pending(client) +for (iv in pending$invites) chat_join(client, iv$channel) +} +} diff --git a/man/chat_relogin.Rd b/man/chat_relogin.Rd new file mode 100644 index 0000000..9146a26 --- /dev/null +++ b/man/chat_relogin.Rd @@ -0,0 +1,26 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{chat_relogin} +\alias{chat_relogin} +\title{Refresh this client's credentials} +\usage{ +chat_relogin(client, ...) +} +\arguments{ +\item{client}{A \code{chat_client}.} + +\item{...}{Adapter-specific options.} +} +\value{ +TRUE, invisibly. +} +\description{ +Forces the re-authentication that adapters otherwise perform on +demand. The refreshed credentials stay inside the client. +} +\details{ +The default throws rather than returning quietly. "I could not +refresh" and "there was nothing to refresh" look identical to a +caller that gets FALSE, and the first means the next call will fail +with a stale token. + +} diff --git a/man/chat_set_identity.Rd b/man/chat_set_identity.Rd new file mode 100644 index 0000000..7fa1041 --- /dev/null +++ b/man/chat_set_identity.Rd @@ -0,0 +1,33 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{chat_set_identity} +\alias{chat_set_identity} +\title{Set this client's persistent identity} +\usage{ +chat_set_identity(client, display, ...) +} +\arguments{ +\item{client}{A \code{chat_client}.} + +\item{display}{New display name.} + +\item{...}{Adapter-specific options.} +} +\value{ +TRUE if the identity was changed, invisibly. +} +\description{ +The account's own display name, as everyone in every channel sees it +until it is changed again. Distinct from \code{\link{chat_send}}'s +\code{identity} argument, which decorates a single message on +platforms that allow it. +} +\details{ +Owning this matters beyond tidiness. On Matrix the rename is an +authenticated call that can rotate the access token underneath the +caller, and a consumer that made that call itself had to notice the +rotation and get the new token back into its client -- usually via +whatever file both of them happened to share. Behind the contract the +rotation lands in the client that performed it, and nothing outside +has to know it happened. + +} From d685c785702d7303a82fe081307736131a3a2703 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Fri, 7 Aug 2026 14:17:23 -0500 Subject: [PATCH 2/5] Let a kind spelled as a Matrix msgtype through as itself The contract has three kinds and no word for an image or a file, so anything unrecognized fell to m.text -- which posts a text message whose body is a filename. A caller that names a Matrix type explicitly now gets it, which is what stops corteza reaching around the contract to mx_send_text() for the msgtypes its exported matrix_send() documents. --- R/matrix.R | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/R/matrix.R b/R/matrix.R index 0855571..c04f979 100644 --- a/R/matrix.R +++ b/R/matrix.R @@ -543,10 +543,20 @@ chat_send.chat_matrix <- function(client, channel, text, media_ids <- c(media_ids, as.character(event)) } } + # The contract's three kinds, plus a documented way past them: a + # kind already spelled as a Matrix msgtype goes out as itself. The + # contract has no word for an image or a file, and mapping those to + # "m.text" -- which is what the else branch below does to anything + # unrecognized -- posts a text message whose body is a filename. + # Better a caller that names a Matrix type explicitly than a caller + # forced around the contract entirely to send one. msgtype <- if (identical(kind, "notice")) { "m.notice" } else if (identical(kind, "emote")) { "m.emote" + } else if (is.character(kind) && length(kind) == 1L && + startsWith(kind, "m.")) { + kind } else { "m.text" } From ff9bb2e342915c3776646cee2840c54e4116d987 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Fri, 7 Aug 2026 14:20:36 -0500 Subject: [PATCH 3/5] Reload the config even when the rename throws The comment said unconditionally; the code returned early. A relogin inside the rename persists a refreshed token before retrying, so a retry that then fails has still rotated it -- the live token is on disk and the client is holding the rejected one. Nothing relogins on a send, so the next reply dies in a best-effort tryCatch with nothing logged. Found by mutation testing: cutting the reload out entirely left the suite green, which is what sent me looking for the case that would have covered it. --- R/matrix-config.R | 64 ++++++++++++++++++---------- inst/tinytest/test_matrix_mxclient.R | 63 +++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 23 deletions(-) diff --git a/R/matrix-config.R b/R/matrix-config.R index 76cf540..4715e6c 100644 --- a/R/matrix-config.R +++ b/R/matrix-config.R @@ -197,32 +197,50 @@ chat_relogin.chat_matrix <- function(client, ...) { #' @export chat_set_identity.chat_matrix <- function(client, display, ...) { fn <- client$identity_fn %||% mx.client::mx_set_displayname - fn(client$env$mx, display) - # mx_set_displayname() wraps itself in mx_with_relogin(), which - # persists a refreshed config before retrying and then returns only - # TRUE -- discarding the client it refreshed. So the live token may - # now be on disk and not in memory, and the only way to pick it up - # is to read it back. + # The failure is held, not swallowed: the reload below has to run + # either way, and the caller still has to hear that the rename did + # not happen. # - # Unconditionally, not on success: a relogin whose retry then failed - # (rate limit, transient 5xx) still rotated the token and still - # wrote it. Reloading only on success is how the following send goes - # out on the token the homeserver has already rejected. + # mx_set_displayname() wraps itself in mx_with_relogin(), which + # persists a refreshed config *before* retrying and then returns only + # TRUE, discarding the client it refreshed. So a relogin whose retry + # then failed -- rate limit, transient 5xx -- has still rotated the + # token and still written it, and the live one is on disk while this + # client holds the rejected one. Reloading only on success is exactly + # how the next send goes out on a token the homeserver already + # refused, and it fails silently, because nothing in this stack + # relogins on a send. + err <- NULL + tryCatch(fn(client$env$mx, display), error = function(e) err <<- e) + matrix_reload_into(client) + if (!is.null(err)) { + stop(err) + } + invisible(TRUE) +} + +# Re-read the config from wherever this client's was loaded and adopt it. +# A client with no app and no path has nothing to re-read -- it was +# handed a config directly and nobody is persisting it -- and a read that +# fails leaves what we already had, which is no worse than not looking. +matrix_reload_into <- function(client) { path <- attr(client$env$mx, "path") app <- attr(client$env$mx, "app") %||% client$app - if (!is.null(path) || !is.null(app)) { - args <- list() - if (!is.null(app)) { - args$app <- app - } - if (!is.null(path)) { - args$path <- path - } - fresh <- tryCatch(do.call(mx.client::mx_client_load, args), - error = function(e) NULL) - if (!is.null(fresh)) { - client$env$mx <- fresh - } + if (is.null(path) && is.null(app)) { + return(invisible(FALSE)) + } + args <- list() + if (!is.null(app)) { + args$app <- app + } + if (!is.null(path)) { + args$path <- path + } + fresh <- tryCatch(do.call(mx.client::mx_client_load, args), + error = function(e) NULL) + if (is.null(fresh)) { + return(invisible(FALSE)) } + client$env$mx <- fresh invisible(TRUE) } diff --git a/inst/tinytest/test_matrix_mxclient.R b/inst/tinytest/test_matrix_mxclient.R index 4a94f78..29a4d7c 100644 --- a/inst/tinytest/test_matrix_mxclient.R +++ b/inst/tinytest/test_matrix_mxclient.R @@ -934,3 +934,66 @@ local({ .send = function(...) "$1", .media = function(...) NULL) expect_identical(chat_whoami(cl)$id, "@bot:ex") }) + +# ---- chat_set_identity picks up a rotation it did not perform ---- +# The rename wraps itself in a relogin that persists a refreshed token +# before retrying, then returns only TRUE and discards the client it +# refreshed. So the live token can be on disk and not in memory, and the +# only way to have it is to read it back. This is what lets a consumer +# hold one client across a rename instead of rebuilding one per send off +# a file they both happen to write. +local({ + dir <- tempfile("ident-") + dir.create(dir) + on.exit(unlink(dir, recursive = TRUE), add = TRUE) + path <- file.path(dir, "matrix.json") + + cfg <- chat_config(list(server = "https://ex.invalid", user = "bot", + password = "pw", token = "tok", + user_id = "@bot:ex", device_id = "DEV1"), + path = path) + chat_config_save(cfg) + + cl <- chat_matrix(mx = cfg, .sync = function(...) NULL, + .extract = function(...) list(), + .send = function(...) "$1", .media = function(...) NULL, + .identity = function(client, name, ...) { + # What a relogin inside the rename does: write + # the new token, report only success. + on_disk <- chat_matrix_config(path = path) + on_disk$token <- "rotated" + chat_config_save(on_disk) + invisible(TRUE) + }) + expect_identical(cl$env$mx$token, "tok") + chat_set_identity(cl, "corteza [opus]") + expect_identical(cl$env$mx$token, "rotated") +}) + +# And unconditionally, not only when the rename reported success. A +# relogin whose retry then failed -- rate limit, transient 5xx -- still +# rotated the token and still wrote it. Reloading only on success is how +# the next send goes out on the token the homeserver already rejected. +local({ + dir <- tempfile("ident-") + dir.create(dir) + on.exit(unlink(dir, recursive = TRUE), add = TRUE) + path <- file.path(dir, "matrix.json") + cfg <- chat_config(list(server = "https://ex.invalid", user = "bot", + password = "pw", token = "tok", + user_id = "@bot:ex", device_id = "DEV1"), + path = path) + chat_config_save(cfg) + + cl <- chat_matrix(mx = cfg, .sync = function(...) NULL, + .extract = function(...) list(), + .send = function(...) "$1", .media = function(...) NULL, + .identity = function(client, name, ...) { + on_disk <- chat_matrix_config(path = path) + on_disk$token <- "rotated" + chat_config_save(on_disk) + stop("M_LIMIT_EXCEEDED") + }) + expect_error(chat_set_identity(cl, "x"), "M_LIMIT_EXCEEDED") + expect_identical(cl$env$mx$token, "rotated") +}) From b14d58653ecc4f68fc24107ed0f9887c51ce69f4 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Fri, 7 Aug 2026 14:58:23 -0500 Subject: [PATCH 4/5] chat_history() pages by an opaque cursor, and returns one The blocker: `before` was documented as a message id, and Matrix's /messages does not take one. Its `from` is a pagination token out of a previous response, so handing it an event id does not page from that event. The contract also gave a consumer no way to continue at all -- one page and no token. There is no id that means the same thing on both reference transports: Slack pages by its own next_cursor, Matrix by `end`. So the contract does what it already does for chat_poll() -- the token is the adapter's, and a consumer only ever hands back what it was given. chat_history() now returns list(messages, cursor). NULL cursor means the channel has no more history behind this page, which on Matrix is `end` being omitted rather than the chunk being empty: a window can be all state events and still have conversation behind it. On Slack it is next_cursor coming back as "", which handed back to conversations.history is an error rather than a no-op. Loopback pages by a count rather than a message id, deliberately. It is what a new adapter gets read as an example, and one that paged by id would teach a contract the reference transport cannot honour. --- DESCRIPTION | 2 +- R/contract.R | 29 +++++++++++---- R/loopback.R | 31 ++++++++++------ R/matrix.R | 16 ++++++--- R/slack.R | 20 ++++++++--- inst/tinytest/test_contract.R | 62 ++++++++++++++++++++++++++++++++ inst/tinytest/test_matrix.R | 42 +++++++++++++++++----- inst/tinytest/test_slack.R | 68 +++++++++++++++++++++++++++++++++++ man/chat_history.Rd | 28 ++++++++++++--- 9 files changed, 257 insertions(+), 41 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index d62866e..5cf6f8b 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: chat.api Type: Package Title: Transport-Agnostic Chat Contract for R Agents -Version: 0.0.1.16 +Version: 0.0.1.17 Date: 2026-08-05 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), diff --git a/R/contract.R b/R/contract.R index 6fb301d..bd76414 100644 --- a/R/contract.R +++ b/R/contract.R @@ -571,10 +571,23 @@ chat_channels.default <- function(client, ...) { #' @param client A \code{chat_client}. #' @param channel Channel/room identifier. #' @param limit Maximum messages to return. -#' @param before Return messages older than this message id, for paging -#' backwards; NULL starts from the most recent. +#' @param cursor Opaque continuation token from a previous call's +#' \code{cursor}, to read the page before it; NULL starts from the most +#' recent. #' @param ... Adapter-specific options. -#' @return A list of \code{\link{chat_message}}, oldest first. +#' @return A list with \code{messages} (list of \code{\link{chat_message}}, +#' oldest first) and \code{cursor} (opaque; pass it back to read +#' further into the past, NULL when the channel has no more history). +#' +#' @section The cursor is opaque, like chat_poll's: +#' Not a message id. This started out taking one and it was wrong on the +#' reference transport: Matrix's \code{/messages} takes a pagination +#' token from a previous response, and handing it an event id does not +#' page from that event -- it fails, or worse, silently returns the wrong +#' window. Slack pages by its own \code{next_cursor}. There is no id that +#' means the same thing on both, so the contract does what it already +#' does for \code{\link{chat_poll}}: the token is the adapter's, and a +#' consumer only ever passes back what it was given. #' #' @section Order: #' Chronological, oldest first, whatever the platform's native direction @@ -584,6 +597,10 @@ chat_channels.default <- function(client, ...) { #' consumer, and a consumer that gets it wrong produces a transcript #' that reads backwards without erroring. #' +#' Note that pages run backwards while each page runs forwards: call it +#' twice and the second page's messages all precede the first page's. +#' A consumer assembling a full transcript prepends. +#' #' @section Overlap with chat_poll: #' The same message can arrive from both, and adapters must return the #' same \code{id} for it either way. That id is the only thing a consumer @@ -592,14 +609,14 @@ chat_channels.default <- function(client, ...) { #' @examples #' cl <- chat_loopback() #' chat_send(cl, "general", "hello") -#' chat_history(cl, "general") +#' chat_history(cl, "general")$messages #' @export -chat_history <- function(client, channel, limit = 50L, before = NULL, ...) { +chat_history <- function(client, channel, limit = 50L, cursor = NULL, ...) { UseMethod("chat_history") } #' @export -chat_history.default <- function(client, channel, limit = 50L, before = NULL, +chat_history.default <- function(client, channel, limit = 50L, cursor = NULL, ...) { stop("chat_history() is not supported by this adapter (", paste(class(client), collapse = "/"), diff --git a/R/loopback.R b/R/loopback.R index 122bc88..73b3822 100644 --- a/R/loopback.R +++ b/R/loopback.R @@ -82,25 +82,34 @@ chat_channels.chat_loopback <- function(client, ...) { #' @export chat_history.chat_loopback <- function(client, channel, limit = 50L, - before = NULL, ...) { + cursor = NULL, ...) { log <- client$env$log keep <- vapply(log, function(m) identical(m$channel, channel), logical(1)) log <- log[keep] - if (!is.null(before)) { - pos <- which(vapply(log, function(m) identical(m$id, before), - logical(1))) - if (length(pos)) { - if (pos[[1L]] > 1L) { - log <- log[seq_len(pos[[1L]] - 1L)] - } else { - log <- list() - } + # The cursor is a count of how many of this channel's messages the + # caller has already seen from the end. An integer, deliberately + # opaque: the reference adapter is what a new adapter is read as an + # example, and one that paged by message id would teach the wrong + # thing -- Matrix cannot do that at all. + seen <- if (is.null(cursor)) { + 0L + } else { + as.integer(cursor) + } + if (seen > 0L) { + log <- if (seen >= length(log)) { + list() + } else { + log[seq_len(length(log) - seen)] } } # The tail, still oldest-first. limit trims the far end, not the near # one: "the last 20 messages" means the 20 most recent. if (length(log) > limit) { log <- log[seq.int(length(log) - limit + 1L, length(log))] + nxt <- seen + limit + } else { + nxt <- NULL } - log + list(messages = log, cursor = nxt) } diff --git a/R/matrix.R b/R/matrix.R index c04f979..ce7f5db 100644 --- a/R/matrix.R +++ b/R/matrix.R @@ -809,12 +809,16 @@ chat_channels.chat_matrix <- function(client, ...) { #' @export chat_history.chat_matrix <- function(client, channel, limit = 50L, - before = NULL, ...) { + cursor = NULL, ...) { fn <- client$history_fn %||% mx.api::mx_messages sess <- mx.client::mx_client_session(client$env$mx) args <- list(sess, channel, dir = "b", limit = as.integer(limit)) - if (!is.null(before)) { - args$from <- before + if (!is.null(cursor)) { + # /messages `from` is a pagination token out of a previous + # response, not an event id. Handing it an event id does not page + # from that event -- which is exactly why the contract's cursor + # is opaque and comes from here rather than off a message. + args$from <- cursor } res <- do.call(fn, args) chunk <- res$chunk %||% list() @@ -850,7 +854,11 @@ chat_history.chat_matrix <- function(client, channel, limit = 50L, use.names = FALSE), raw = ev) } - out + # `end` continues further back. The spec omits it when there is + # nothing older, which is how a consumer paging to the start of a + # room knows to stop -- an empty chunk is not the signal, because a + # window can be all state events and still have history behind it. + list(messages = out, cursor = res$end) } #' @export diff --git a/R/slack.R b/R/slack.R index 6941fde..5142bd7 100644 --- a/R/slack.R +++ b/R/slack.R @@ -463,14 +463,17 @@ chat_channels.chat_slack <- function(client, ...) { #' @export chat_history.chat_slack <- function(client, channel, limit = 50L, - before = NULL, ...) { + cursor = NULL, ...) { api <- client$api_fn %||% slackr::call_slack_api args <- list("/api/conversations.history", .method = "GET", token = client$token, channel = sub("^#", "", channel), limit = as.integer(limit)) - if (!is.null(before)) { - args$latest <- before - args$inclusive <- FALSE + if (!is.null(cursor)) { + # Slack's own cursor, not a `latest` timestamp. Both page + # backwards here and a message ts would even work, but the + # contract's token has to mean one thing across adapters, and on + # Matrix it cannot be a message id at all. + args$cursor <- cursor } body <- slack_body(do.call(api, args)) slack_stop_for_error(body, "conversations.history") @@ -496,7 +499,14 @@ chat_history.chat_slack <- function(client, channel, limit = 50L, markup = "plain", kind = "message", thread = m$thread_ts, raw = m) } - out + # Slack sends "" rather than omitting next_cursor when there is no + # more, and an empty-string cursor handed back to conversations.history + # is an error rather than a no-op. + nxt <- body$response_metadata$next_cursor + if (is.null(nxt) || !nzchar(nxt)) { + nxt <- NULL + } + list(messages = out, cursor = nxt) } #' @export diff --git a/inst/tinytest/test_contract.R b/inst/tinytest/test_contract.R index f8d9a08..abddf03 100644 --- a/inst/tinytest/test_contract.R +++ b/inst/tinytest/test_contract.R @@ -207,3 +207,65 @@ expect_error(chat_addressed(nothing, sender = "a", body = "b", ts = Sys.time())), "not supported by this adapter") + +# ---- chat_history paging on the reference adapter ---- +# The loopback adapter is what a new adapter gets read as an example, so +# its cursor is deliberately not a message id. Matrix cannot page by one +# at all, and a reference implementation that did would teach the wrong +# contract. +local({ + cl <- chat_loopback() + for (i in 1:5) { + chat_send(cl, "general", sprintf("m%d", i)) + } + chat_send(cl, "other", "elsewhere") + + all <- chat_history(cl, "general") + expect_identical(length(all$messages), 5L) + # Oldest first, and only this channel's. + expect_identical(vapply(all$messages, function(m) m$body, character(1)), + c("m1", "m2", "m3", "m4", "m5")) + # Nothing left behind it, so no continuation. + expect_null(all$cursor) + + # limit takes the most recent, not the first stored. + page1 <- chat_history(cl, "general", limit = 2L) + expect_identical(vapply(page1$messages, function(m) m$body, character(1)), + c("m4", "m5")) + expect_false(is.null(page1$cursor)) + + # Pages run backwards while each page runs forwards. + page2 <- chat_history(cl, "general", limit = 2L, cursor = page1$cursor) + expect_identical(vapply(page2$messages, function(m) m$body, character(1)), + c("m2", "m3")) + page3 <- chat_history(cl, "general", limit = 2L, cursor = page2$cursor) + expect_identical(vapply(page3$messages, function(m) m$body, character(1)), + "m1") + # The start of the channel: nothing more to ask for. + expect_null(page3$cursor) + + # Paging past the start is empty, not an error and not a wrap-around. + past <- chat_history(cl, "general", limit = 2L, cursor = 99L) + expect_identical(past$messages, list()) + expect_null(past$cursor) + + # Every page put together is the whole channel, once. + walked <- character() + cur <- NULL + repeat { + pg <- chat_history(cl, "general", limit = 2L, cursor = cur) + walked <- c(vapply(pg$messages, function(m) m$body, character(1)), + walked) + cur <- pg$cursor + if (is.null(cur)) { + break + } + } + expect_identical(walked, c("m1", "m2", "m3", "m4", "m5")) +}) + +# The generic's signature is the contract. `before` took a message id +# once, which Matrix's /messages cannot use -- it wants a pagination +# token out of a previous response. +expect_true("cursor" %in% names(formals(chat_history))) +expect_false("before" %in% names(formals(chat_history))) diff --git a/inst/tinytest/test_matrix.R b/inst/tinytest/test_matrix.R index c0344bd..8ef0de2 100644 --- a/inst/tinytest/test_matrix.R +++ b/inst/tinytest/test_matrix.R @@ -1599,7 +1599,7 @@ local({ hev("$2", "second", ts = 2000), hev("$1", "first", ts = 1000))) }) - h <- chat_history(cl, "!a:ex", limit = 3L) + h <- chat_history(cl, "!a:ex", limit = 3L)$messages expect_identical(seen$room_id, "!a:ex") expect_identical(seen$dir, "b") expect_identical(seen$limit, 3L) @@ -1616,16 +1616,40 @@ local({ }) local({ - # `before` pages backwards from a known id, and only then. + # The cursor is /messages' own `from` token, and it comes back out as + # `end`. Not a message id: handing an event id to /messages does not + # page from that event, so a contract that promised ids would be + # wrong on the reference transport. seen <- NULL cl <- seam_client(.history = function(session, room_id, ...) { seen <<- list(...) - list(chunk = list()) + list(chunk = list(hev("$1", "one")), start = "t1", end = "t2") }) - chat_history(cl, "!a:ex") + first <- chat_history(cl, "!a:ex") expect_false("from" %in% names(seen)) - chat_history(cl, "!a:ex", before = "$9") - expect_identical(seen$from, "$9") + expect_identical(first$cursor, "t2") + chat_history(cl, "!a:ex", cursor = first$cursor) + expect_identical(seen$from, "t2") +}) + +# No `end` means no more history. The spec omits it at the start of a +# room, and that -- not an empty chunk -- is the stop signal: a window +# can be all state events and still have conversation behind it. +local({ + cl <- seam_client(.history = function(...) { + list(chunk = list(list(type = "m.room.member", event_id = "$m"))) + }) + res <- chat_history(cl, "!a:ex") + expect_identical(res$messages, list()) + expect_null(res$cursor) +}) +local({ + cl <- seam_client(.history = function(...) { + list(chunk = list(), end = "t9") + }) + # Nothing in this window, but the room has more behind it. A consumer + # that stopped on the empty page would lose the rest. + expect_identical(chat_history(cl, "!a:ex")$cursor, "t9") }) local({ @@ -1636,7 +1660,7 @@ local({ list(chunk = list(hev("$2", "cat.png", msgtype = "m.image"), hev("$1", "look"))) }) - h <- chat_history(cl, "!a:ex") + h <- chat_history(cl, "!a:ex")$messages expect_identical(length(h), 1L) expect_identical(h[[1L]]$id, "$1") }) @@ -1650,7 +1674,7 @@ local({ list(chunk = list(list(type = "m.room.member", event_id = "$m"), hev("$1", "look"))) }) - expect_identical(length(chat_history(cl, "!a:ex")), 1L) + expect_identical(length(chat_history(cl, "!a:ex")$messages), 1L) }) local({ @@ -1660,7 +1684,7 @@ local({ list(chunk = list(hev("$1", "mine", sender = "@bot:ex", mentions = "@ann:ex"))) }) - h <- chat_history(cl, "!a:ex") + h <- chat_history(cl, "!a:ex")$messages expect_true(h[[1L]]$self) expect_identical(h[[1L]]$mentions, "@ann:ex") }) diff --git a/inst/tinytest/test_slack.R b/inst/tinytest/test_slack.R index 57a72c7..eee2dca 100644 --- a/inst/tinytest/test_slack.R +++ b/inst/tinytest/test_slack.R @@ -465,3 +465,71 @@ local({ }) expect_true(chat_capabilities(slack_api_client(function(...) NULL))$whoami) + +# ---- History paging ---- +local({ + seen <- NULL + cl <- slack_api_client(function(path, ..., .method, token) { + seen <<- c(list(path = path), list(...)) + list(ok = TRUE, + messages = list( + list(ts = "300.0", user = "U1", text = "third"), + list(ts = "200.0", user = "U1", text = "second"), + list(ts = "100.0", user = "U1", text = "first")), + response_metadata = list(next_cursor = "c2")) + }) + res <- chat_history(cl, "#lab", limit = 3L) + expect_identical(seen$path, "/api/conversations.history") + expect_identical(seen$channel, "lab") + expect_identical(seen$limit, 3L) + # Slack pages backwards too; the contract promises the other order. + expect_identical(vapply(res$messages, function(m) m$body, character(1)), + c("first", "second", "third")) + expect_identical(res$cursor, "c2") + # Slack's own cursor, passed back as a cursor -- not as `latest`. A + # message ts would work here and not on Matrix, and the contract's + # token has to mean one thing across adapters. + chat_history(cl, "lab", cursor = "c2") + expect_identical(seen$cursor, "c2") + expect_false("latest" %in% names(seen)) +}) + +# Slack sends "" rather than omitting next_cursor at the end of a +# channel, and handing "" back to conversations.history is an error +# rather than a no-op -- so it has to become NULL here. +local({ + cl <- slack_api_client(function(...) { + list(ok = TRUE, messages = list(list(ts = "1.0", user = "U1", + text = "only")), + response_metadata = list(next_cursor = "")) + }) + expect_null(chat_history(cl, "lab")$cursor) +}) +local({ + cl <- slack_api_client(function(...) { + list(ok = TRUE, messages = list()) + }) + expect_null(chat_history(cl, "lab")$cursor) + expect_identical(chat_history(cl, "lab")$messages, list()) +}) + +# Joins, leaves and channel renames carry a subtype and are not +# conversation. +local({ + cl <- slack_api_client(function(...) { + list(ok = TRUE, messages = list( + list(ts = "2.0", user = "U1", text = "joined", + subtype = "channel_join"), + list(ts = "1.0", user = "U1", text = "real"))) + }) + res <- chat_history(cl, "lab") + expect_identical(length(res$messages), 1L) + expect_identical(res$messages[[1L]]$body, "real") +}) + +# A refused call raises rather than reporting an empty channel. Slack +# says no in the body with HTTP 200, so "no messages" and "you may not +# read this channel" look identical without the check. +expect_error(chat_history(slack_api_client(function(...) { + list(ok = FALSE, error = "channel_not_found") + }), "lab"), "channel_not_found") diff --git a/man/chat_history.Rd b/man/chat_history.Rd index c5a3c18..9107be1 100644 --- a/man/chat_history.Rd +++ b/man/chat_history.Rd @@ -3,7 +3,7 @@ \alias{chat_history} \title{Read a channel's recent messages} \usage{ -chat_history(client, channel, limit = 50L, before = NULL, ...) +chat_history(client, channel, limit = 50L, cursor = NULL, ...) } \arguments{ \item{client}{A \code{chat_client}.} @@ -12,18 +12,32 @@ chat_history(client, channel, limit = 50L, before = NULL, ...) \item{limit}{Maximum messages to return.} -\item{before}{Return messages older than this message id, for paging -backwards; NULL starts from the most recent.} +\item{cursor}{Opaque continuation token from a previous call's +\code{cursor}, to read the page before it; NULL starts from the most +recent.} \item{...}{Adapter-specific options.} } \value{ -A list of \code{\link{chat_message}}, oldest first. +A list with \code{messages} (list of \code{\link{chat_message}}, + oldest first) and \code{cursor} (opaque; pass it back to read + further into the past, NULL when the channel has no more history). } \description{ Independent of the poll cursor: a restarted process uses this to recover the context it lost, and asking for it must not move the cursor or consume anything. +} +\section{The cursor is opaque, like chat_poll's}{ +Not a message id. This started out taking one and it was wrong on the +reference transport: Matrix's \code{/messages} takes a pagination +token from a previous response, and handing it an event id does not +page from that event -- it fails, or worse, silently returns the wrong +window. Slack pages by its own \code{next_cursor}. There is no id that +means the same thing on both, so the contract does what it already +does for \code{\link{chat_poll}}: the token is the adapter's, and a +consumer only ever passes back what it was given. + } \section{Order}{ Chronological, oldest first, whatever the platform's native direction @@ -33,6 +47,10 @@ a transcript has to flip it. One flip in the adapter beats one per consumer, and a consumer that gets it wrong produces a transcript that reads backwards without erroring. +Note that pages run backwards while each page runs forwards: call it +twice and the second page's messages all precede the first page's. +A consumer assembling a full transcript prepends. + } \section{Overlap with chat_poll}{ The same message can arrive from both, and adapters must return the @@ -43,5 +61,5 @@ it routinely cover the same events. \examples{ cl <- chat_loopback() chat_send(cl, "general", "hello") -chat_history(cl, "general") +chat_history(cl, "general")$messages } From 2f49268116d57822b763af2c1c016865ac855129 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Fri, 7 Aug 2026 15:25:35 -0500 Subject: [PATCH 5/5] Bound the page-walk loop The cursor-ignored mutation made it spin forever instead of failing. A hung suite is a worse report than a wrong one: no line number, no diff, just a runner that never finishes -- and it cost two five-minute timeouts to work out which mutation was responsible. --- inst/tinytest/test_contract.R | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/inst/tinytest/test_contract.R b/inst/tinytest/test_contract.R index abddf03..a20efe4 100644 --- a/inst/tinytest/test_contract.R +++ b/inst/tinytest/test_contract.R @@ -252,7 +252,13 @@ local({ # Every page put together is the whole channel, once. walked <- character() cur <- NULL - repeat { + # Bounded, not a repeat. A cursor that fails to advance makes this + # loop forever, and a hung suite is a worse failure report than a + # wrong one: no line number, no diff, just a runner that never + # finishes. Ten is generous for five messages two at a time, and + # cur being non-NULL at the end is what says the walk did not + # terminate on its own. + for (i in seq_len(10L)) { pg <- chat_history(cl, "general", limit = 2L, cursor = cur) walked <- c(vapply(pg$messages, function(m) m$body, character(1)), walked) @@ -261,6 +267,7 @@ local({ break } } + expect_null(cur) expect_identical(walked, c("m1", "m2", "m3", "m4", "m5")) })