mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(cli): allow explicit top-level messages
Automatic reply context must not override an intentional channel-root post. Add a top-level send flag, make it suppress ambient reply targets, and teach the harness to use it. Co-authored-by: npub1x4hk035p3p9q39a3fcrd2fe30lpkrhr5dwe0cqzzjphxyyh8m0gsq4vqap <356f67c681884a0897b14e06d527317fc361dc746bb2fc0042906e6212e7dbd1@buzz.block.builderlab.xyz> Signed-off-by: npub1x4hk035p3p9q39a3fcrd2fe30lpkrhr5dwe0cqzzjphxyyh8m0gsq4vqap <356f67c681884a0897b14e06d527317fc361dc746bb2fc0042906e6212e7dbd1@buzz.block.builderlab.xyz> (cherry picked from commit 21e6fdef9c52c71e92d059f6dd2da80d5e8bad21) Signed-off-by: Michael Neale <michael.neale@gmail.com>
This commit is contained in:
committed by
Michael Neale
parent
3fcd7acef9
commit
0ed01d49b8
@@ -1151,8 +1151,8 @@ fn append_reply_instruction(s: &mut String, event_id: &str) {
|
||||
"\nIMPORTANT: For ordinary replies in this turn, use `--reply-to {event_id}` \
|
||||
on `buzz messages send` so the conversation stays threaded. \
|
||||
If the human explicitly asks for a channel-root, top-level, \
|
||||
or broadcast post, send that message without `--reply-to`. \
|
||||
If the requested destination is ambiguous, ask before sending."
|
||||
or broadcast post, send that message with `--top-level` instead of \
|
||||
`--reply-to`. If the requested destination is ambiguous, ask before sending."
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1167,7 +1167,8 @@ fn append_new_thread_reply_instruction(s: &mut String, event_id: &str) {
|
||||
this turn, use `--reply-to {event_id}` on `buzz messages send` — the \
|
||||
triggering message is the thread root. Do NOT reply into any other \
|
||||
(older) thread. If the human explicitly asks for a channel-root, \
|
||||
top-level, or broadcast post, send that message without `--reply-to`."
|
||||
top-level, or broadcast post, send that message with `--top-level` \
|
||||
instead of `--reply-to`."
|
||||
));
|
||||
}
|
||||
|
||||
@@ -3908,8 +3909,8 @@ mod tests {
|
||||
"channel thread reply should describe reply-to as the default"
|
||||
);
|
||||
assert!(
|
||||
prompt.contains("send that message without `--reply-to`"),
|
||||
"channel thread reply should allow explicit channel-root/top-level requests"
|
||||
prompt.contains("send that message with `--top-level`"),
|
||||
"channel thread reply should teach the automatic-context opt-out"
|
||||
);
|
||||
assert!(
|
||||
!prompt.contains("Do not broadcast to the channel"),
|
||||
@@ -3983,6 +3984,10 @@ mod tests {
|
||||
prompt.contains("new top-level message"),
|
||||
"top-level human message should use the new-thread instruction"
|
||||
);
|
||||
assert!(
|
||||
prompt.contains("send that message with `--top-level`"),
|
||||
"new-thread instruction should teach the automatic-context opt-out"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -476,15 +476,20 @@ pub struct SendMessageParams {
|
||||
pub content: String,
|
||||
pub kind: Option<u16>,
|
||||
pub reply_to: Option<String>,
|
||||
pub top_level: bool,
|
||||
pub broadcast: bool,
|
||||
pub files: Vec<String>,
|
||||
}
|
||||
|
||||
fn reply_to_from_sources(
|
||||
top_level: bool,
|
||||
explicit: Option<String>,
|
||||
env_value: Option<String>,
|
||||
file_path: Option<&std::ffi::OsStr>,
|
||||
) -> Option<String> {
|
||||
if top_level {
|
||||
return None;
|
||||
}
|
||||
explicit
|
||||
.or_else(|| {
|
||||
env_value.and_then(|value| {
|
||||
@@ -499,8 +504,9 @@ fn reply_to_from_sources(
|
||||
})
|
||||
}
|
||||
|
||||
fn automatic_reply_to(explicit: Option<String>) -> Option<String> {
|
||||
fn automatic_reply_to(top_level: bool, explicit: Option<String>) -> Option<String> {
|
||||
reply_to_from_sources(
|
||||
top_level,
|
||||
explicit,
|
||||
std::env::var("BUZZ_REPLY_TO").ok(),
|
||||
std::env::var_os("BUZZ_REPLY_TO_FILE").as_deref(),
|
||||
@@ -517,7 +523,7 @@ pub async fn cmd_send_message(
|
||||
// bugs for agent and human users alike.
|
||||
p.content = read_or_stdin(&p.content)?;
|
||||
validate_content_size(&p.content)?;
|
||||
p.reply_to = automatic_reply_to(p.reply_to);
|
||||
p.reply_to = automatic_reply_to(p.top_level, p.reply_to);
|
||||
if let Some(ref r) = p.reply_to {
|
||||
validate_hex64(r)?;
|
||||
}
|
||||
@@ -791,6 +797,7 @@ pub async fn dispatch(
|
||||
content,
|
||||
kind,
|
||||
reply_to,
|
||||
top_level,
|
||||
broadcast,
|
||||
files,
|
||||
} => {
|
||||
@@ -801,6 +808,7 @@ pub async fn dispatch(
|
||||
content,
|
||||
kind,
|
||||
reply_to,
|
||||
top_level,
|
||||
broadcast,
|
||||
files,
|
||||
},
|
||||
@@ -928,6 +936,7 @@ mod tests {
|
||||
std::fs::write(file.path(), ID_B).unwrap();
|
||||
assert_eq!(
|
||||
reply_to_from_sources(
|
||||
false,
|
||||
Some(ID_A.into()),
|
||||
Some(ID_B.into()),
|
||||
Some(file.path().as_os_str()),
|
||||
@@ -943,6 +952,7 @@ mod tests {
|
||||
std::fs::write(file.path(), ID_A).unwrap();
|
||||
assert_eq!(
|
||||
reply_to_from_sources(
|
||||
false,
|
||||
None,
|
||||
Some(format!(" {ID_B}\n")),
|
||||
Some(file.path().as_os_str()),
|
||||
@@ -957,16 +967,36 @@ mod tests {
|
||||
let file = tempfile::NamedTempFile::new().unwrap();
|
||||
std::fs::write(file.path(), format!(" {ID_A}\n")).unwrap();
|
||||
assert_eq!(
|
||||
reply_to_from_sources(None, Some(" ".into()), Some(file.path().as_os_str()))
|
||||
.as_deref(),
|
||||
reply_to_from_sources(
|
||||
false,
|
||||
None,
|
||||
Some(" ".into()),
|
||||
Some(file.path().as_os_str()),
|
||||
)
|
||||
.as_deref(),
|
||||
Some(ID_A)
|
||||
);
|
||||
std::fs::write(file.path(), "").unwrap();
|
||||
assert_eq!(
|
||||
reply_to_from_sources(None, None, Some(file.path().as_os_str())),
|
||||
reply_to_from_sources(false, None, None, Some(file.path().as_os_str())),
|
||||
None
|
||||
);
|
||||
assert_eq!(reply_to_from_sources(false, None, None, None), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn top_level_suppresses_explicit_environment_and_file_reply_targets() {
|
||||
let file = tempfile::NamedTempFile::new().unwrap();
|
||||
std::fs::write(file.path(), ID_A).unwrap();
|
||||
assert_eq!(
|
||||
reply_to_from_sources(
|
||||
true,
|
||||
Some(ID_A.into()),
|
||||
Some(ID_B.into()),
|
||||
Some(file.path().as_os_str()),
|
||||
),
|
||||
None
|
||||
);
|
||||
assert_eq!(reply_to_from_sources(None, None, None), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -361,8 +361,11 @@ pub enum MessagesCmd {
|
||||
#[arg(long)]
|
||||
kind: Option<u16>,
|
||||
/// Event ID to reply to (creates a thread)
|
||||
#[arg(long)]
|
||||
#[arg(long, conflicts_with = "top_level")]
|
||||
reply_to: Option<String>,
|
||||
/// Send at channel root, ignoring automatic reply context
|
||||
#[arg(long, default_value_t = false, conflicts_with = "reply_to")]
|
||||
top_level: bool,
|
||||
/// Also publish to the Nostr network
|
||||
#[arg(long, default_value_t = false)]
|
||||
broadcast: bool,
|
||||
@@ -1840,6 +1843,45 @@ mod tests {
|
||||
assert!(Cli::try_parse_from(["buzz", "users", "set-status", "--clear"]).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn message_send_top_level_is_explicit_and_conflicts_with_reply_to() {
|
||||
let cli = Cli::try_parse_from([
|
||||
"buzz",
|
||||
"messages",
|
||||
"send",
|
||||
"--channel",
|
||||
"00000000-0000-0000-0000-000000000000",
|
||||
"--content",
|
||||
"announcement",
|
||||
"--top-level",
|
||||
])
|
||||
.expect("--top-level should parse");
|
||||
let Cmd::Messages(MessagesCmd::Send {
|
||||
top_level,
|
||||
reply_to,
|
||||
..
|
||||
}) = cli.command
|
||||
else {
|
||||
panic!("expected messages send");
|
||||
};
|
||||
assert!(top_level);
|
||||
assert!(reply_to.is_none());
|
||||
|
||||
let conflict = Cli::try_parse_from([
|
||||
"buzz",
|
||||
"messages",
|
||||
"send",
|
||||
"--channel",
|
||||
"00000000-0000-0000-0000-000000000000",
|
||||
"--content",
|
||||
"announcement",
|
||||
"--top-level",
|
||||
"--reply-to",
|
||||
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
|
||||
]);
|
||||
assert!(conflict.is_err(), "explicit destinations must not conflict");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_inventory_is_stable() {
|
||||
let expected_groups: Vec<&str> = vec![
|
||||
|
||||
Reference in New Issue
Block a user