Repository navigation
mx_set_displayname() should return the refreshed client #18
Description
mx_set_displayname() wraps its request in mx_with_relogin(), so a rejected token is refreshed and persisted. But it discards what the wrapper returns:
mx_set_displayname <- function(client, name, save = TRUE) {
...
mx_with_relogin(client, function(cl) {
mx.api::mx_set_displayname(mx_client_session(cl), name)
}, save = save)
invisible(TRUE) # <- the refreshed client is dropped here
}A caller therefore has no way to learn that its client is now stale. It keeps the rejected token in memory while the live one sits on disk.
Why this bit
corteza's model badge renames the bot on every /model switch, so it calls this on a path where token rotation is expected. The acknowledgement that follows the rename went out on the rejected token, and because chat.api::chat_send() does no relogin of its own, the failure died in a best-effort tryCatch with nothing logged. The user saw a /model switch that silently never acknowledged.
It took several passes to find, because the token is copied into a number of derived objects and refreshing one leaves the others stale. corteza now works around it by re-reading its config from disk after any attempted rename:
tryCatch(mx.client::mx_set_displayname(matrix_client(cfg), name),
error = function(e) NULL)
# Re-read whether or not the rename succeeded; disk is authoritative.
invisible(tryCatch(matrix_load_config(), error = function(e) cfg))That works but it is a disk round-trip standing in for a return value.
Proposal
Return the client, matching what mx_with_relogin() already hands back:
res <- mx_with_relogin(client, function(cl) {
mx.api::mx_set_displayname(mx_client_session(cl), name)
cl
}, save = save)
invisible(res)Callers that ignore the value are unaffected, since it is invisible. invisible(TRUE) carries no information a caller can act on, so nothing depends on the current return.
Worth checking at the same time
Two subtleties this surfaced, either of which would make a good doc addition:
-
mx_with_relogin()persists before it retries. So a relogin whose retry then fails (rate limit, transient 5xx) still leaves the live token on disk even though the call threw. Any caller that only adopts a refreshed client on success discards it. That is not obvious from outside and it is worth stating in?mx_with_relogin. -
Other wrappers may have the same shape. Anything else in mx.client that goes through
mx_with_relogin()and returns a bareTRUEhas this problem too, and is worth a sweep.
Activity
TroyHernandez commentedon Aug 5, 2026
Swept the package for the same shape — a
mx_with_relogin()wrapper that returns a bareTRUE— andmx_set_displayname()inR/profile.R:33is the only one. Every other caller ofmx_with_relogin()passes its return value through.So this is a single-site fix, not a pattern to unwind.
TroyHernandez commentedon Aug 5, 2026
Correction to the proposal above: returning the client is necessary but not sufficient.
That works when the retry succeeds. When the retry fails -- rate limit, transient 5xx --
mx_with_relogin()propagates the error, so there is no return value at all. The refreshed client is already persisted to disk at that point, but a caller has no way to reach it through the return.So corteza's unconditional disk reload after any attempted rename is not a workaround to retire once this lands. It is load-bearing on the error path:
Flagging it because the obvious reading of this issue is "mx.client returns the client, so corteza can drop the reload" -- and doing that would reintroduce the original silent-drop bug on exactly the path that is hardest to reproduce.
What a complete fix needs
mx_with_relogin()has to expose the refreshed client on the error path too. Options, roughly in order of how much they disturb the existing contract:list(ok =, value =, client =). Cleanest semantics, most disruptive to existing callers.Worth documenting the persist-before-retry ordering in
?mx_with_relogineither way, since that is the detail that makes the error path recoverable at all and it is invisible from outside.