From c26a6fe722dd9517963c0676a81659d92d8781f7 Mon Sep 17 00:00:00 2001 From: theBlanketBaron Date: Fri, 6 Nov 2026 20:14:50 -0500 Subject: [PATCH] fix(shim): tell refuses a nickname two sessions share (MUS-109) Nicknames repeat across restarts, so the first match could send a terminal tell (and rally digest) to a ghost row and report it sent. A nickname that names several sessions is now refused with their ids, as an ambiguous prefix already was. Co-Authored-By: Claude Opus 5.5 --- crates/shim/src/terminal_send.rs | 53 ++++++++++++++++++++++++++++++-- docs/roadmap.md | 6 ++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/crates/shim/src/terminal_send.rs b/crates/shim/src/terminal_send.rs index aab7a77c..437a3cca 100644 --- a/crates/shim/src/terminal_send.rs +++ b/crates/shim/src/terminal_send.rs @@ -59,9 +59,28 @@ fn resolve_targets( .is_some_and(|p| p.eq_ignore_ascii_case(token)) }) .collect(); - let single = agents - .iter() - .find(|s| s.id.0.as_ref() == token || handle(nicknames, s) == *token); + // An exact id names one session; a nickname can name several, since + // nicknames repeat across restarts (a ghost row and its live successor + // were both `mellow-weevil` on 2026-11-06), and sending to the first could + // hand the message to the ghost. + let single = match agents.iter().find(|s| s.id.0.as_ref() == token) { + Some(session) => Some(session), + None => { + let named: Vec<_> = agents + .iter() + .filter(|s| handle(nicknames, s) == *token) + .collect(); + if named.len() > 1 { + let ids: Vec<&str> = named.iter().map(|s| s.id.0.as_ref()).collect(); + return Err(format!( + "`{token}` names {} sessions ({}); use one's id", + named.len(), + ids.join(", ") + )); + } + named.into_iter().next() + } + }; match (team.is_empty(), single) { (false, Some(_)) => { return Err(format!( @@ -327,6 +346,34 @@ mod tests { assert!(resolve_targets(&sessions, &[], &named("catapp")).is_err()); } + #[test] + fn a_nickname_two_sessions_share_is_refused_with_their_ids() { + let sessions = vec![ + session("ghost-0000-0000", None, None), + session("live-1111-1111", None, None), + ]; + let nick = |id: &str| { + Arc::new(SessionNickname { + id: muster_entities::SessionNicknameId(Arc::from(id)), + nickname: "mellow-weevil".into(), + }) + }; + let nicknames = vec![nick("ghost-0000-0000"), nick("live-1111-1111")]; + let err = resolve_targets(&sessions, &nicknames, &named("mellow-weevil")).unwrap_err(); + assert!( + err.contains("ghost-0000-0000") && err.contains("live-1111-1111"), + "{err}" + ); + // Either id still reaches its session. + let one = resolve_targets(&sessions, &nicknames, &named("live-1111-1111")).unwrap(); + assert_eq!(one.len(), 1); + assert_eq!(one[0].id.0.as_ref(), "live-1111-1111"); + // With only one session on that nickname, it resolves to that one. + let one = resolve_targets(&sessions, &nicknames[1..], &named("mellow-weevil")).unwrap(); + assert_eq!(one.len(), 1); + assert_eq!(one[0].id.0.as_ref(), "live-1111-1111"); + } + #[test] fn a_terminal_cant_be_addressed() { assert!(resolve_targets(&roster(), &[], &named("cccccccc-1")).is_err()); diff --git a/docs/roadmap.md b/docs/roadmap.md index f525bf80..dfc3de8f 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -578,6 +578,12 @@ Merged and needing a live run: MUS-36 and MUS-37. message, marks it shown (`claimShown`, MUS-90) and returns its text, which was thrown away; the second message's own push then finds it already shown and skips it. Whether it fails depends on whether the second push lands before that drain. Done: what the drain returns is prompted too. +- [x] **MUS-109: `muster-shim tell` refuses a nickname two sessions share.** From the watchman + (mellow-weevil), 2026-11-06, after the same bug in Marshal's port (marshal#168). Nicknames + repeat across restarts, and today a ghost row and the live watchman were both mellow-weevil, so + `resolve_targets`' first match could hand `rally digest`'s `tell mellow-weevil` to the ghost and + report it sent. Done: an exact id still names one session; a nickname that matches several is + refused with their ids, as an ambiguous prefix already was. - [ ] **MUS-30: Decide the future of the TypeScript plugins.** The operator decides whether to keep, freeze or remove them. Done when: the decision is recorded here, and if they're removed, the repository's `package.json` workspace goes with them.