diff --git a/README.md b/README.md index 9020a37d..d62c7d2d 100644 --- a/README.md +++ b/README.md @@ -377,6 +377,23 @@ st2 agents --json --enrich st2 context read --full ``` +Use one optional key when a sender can retry the same message: + +```sh +check_status | st2 message send \ + --idempotency-key daily-check-2026-07-31 \ + --subject "Daily check" +``` + +st2 stores the key in the normal message frontmatter. It takes a short local +lock and searches the recipient inbox first. It then searches the archive. A +retry returns the first matching filename. It does not create another inbox +file or another DING. If the archived message is deleted, st2 forgets the key. + +This rule applies to one recipient on one local catalog filesystem. It is not a +global exactly-once rule across replicas. Without `--idempotency-key`, `message +send` keeps its existing bytes, filename output, and inbox behavior. + The roster includes retired declarations instead of silently conflating them with runtime presence. Both JSON shapes keep stable `identity` separate from optional `name` and `description`, and contain `retired` plus the declaration's ordered `resources` descriptors. `--enrich` diff --git a/src/ding/mod.rs b/src/ding/mod.rs index 2841edee..83a2f363 100644 --- a/src/ding/mod.rs +++ b/src/ding/mod.rs @@ -1014,6 +1014,7 @@ mod tests { in_reply_to: None, tags: vec![], priority: None, + idempotency_key: None, body: String::new(), } } diff --git a/src/main.rs b/src/main.rs index 2eaf95d7..f7205c3b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -571,6 +571,9 @@ enum MessageCmd { /// Comma-separated tags. #[arg(long, value_delimiter = ',')] tags: Vec, + /// Return the first matching normal message for this local recipient and key. + #[arg(long = "idempotency-key")] + idempotency_key: Option, #[command(flatten)] ctx: MsgCtx, }, @@ -1658,6 +1661,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { subject, in_reply_to, tags, + idempotency_key, ctx, } => { let (root, host) = resolve_ctx(&ctx)?; @@ -1672,6 +1676,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { in_reply_to.as_deref(), &tags, &body, + idempotency_key.as_deref(), )?; println!("{filename}"); Ok(()) @@ -1702,6 +1707,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { Some(&filename), &[], &body, + None, )?; println!("{sent}"); Ok(()) @@ -1793,6 +1799,9 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { if !m.tags.is_empty() { println!("tags: {}", m.tags.join(", ")); } + if let Some(key) = &m.idempotency_key { + println!("idempotency-key: {key}"); + } println!(); print!("{}", m.body); Ok(()) @@ -1847,12 +1856,46 @@ fn send_resolved_message( in_reply_to: Option<&str>, tags: &[String], body: &str, + idempotency_key: Option<&str>, ) -> Result { if std::env::var("ST2_EVAL_REQUESTER").as_deref() == Ok(to) { let inbox = resolve_message_inbox(root, to, host)?; - message::send_to_inbox(&inbox, from, subject, in_reply_to, tags, body) + match idempotency_key { + Some(key) => message::send_idempotent_to_inbox( + &inbox, + from, + subject, + in_reply_to, + tags, + body, + key, + ), + None => message::send_to_inbox(&inbox, from, subject, in_reply_to, tags, body), + } } else { - message::send_to_resolved_inbox(root, to, host, from, subject, in_reply_to, tags, body) + match idempotency_key { + Some(key) => message::send_idempotent_to_resolved_inbox( + root, + to, + host, + from, + subject, + in_reply_to, + tags, + body, + key, + ), + None => message::send_to_resolved_inbox( + root, + to, + host, + from, + subject, + in_reply_to, + tags, + body, + ), + } } } @@ -1867,6 +1910,8 @@ struct LsItemJson<'a> { in_reply_to: Option<&'a str>, tags: &'a [String], priority: Option<&'a str>, + #[serde(rename = "idempotencyKey", skip_serializing_if = "Option::is_none")] + idempotency_key: Option<&'a str>, #[serde(skip_serializing_if = "Option::is_none")] body: Option<&'a str>, } @@ -1881,6 +1926,7 @@ impl<'a> From<&'a st2::message::Message> for LsItemJson<'a> { in_reply_to: m.in_reply_to.as_deref(), tags: &m.tags, priority: m.priority.as_deref(), + idempotency_key: m.idempotency_key.as_deref(), body: None, } } @@ -1907,6 +1953,8 @@ struct MessageJson<'a> { in_reply_to: Option<&'a str>, tags: &'a [String], priority: Option<&'a str>, + #[serde(rename = "idempotencyKey", skip_serializing_if = "Option::is_none")] + idempotency_key: Option<&'a str>, body: &'a str, } @@ -1920,6 +1968,7 @@ impl<'a> From<&'a st2::message::Message> for MessageJson<'a> { in_reply_to: m.in_reply_to.as_deref(), tags: &m.tags, priority: m.priority.as_deref(), + idempotency_key: m.idempotency_key.as_deref(), body: &m.body, } } diff --git a/src/message.rs b/src/message.rs index 573ea878..37145fbe 100644 --- a/src/message.rs +++ b/src/message.rs @@ -14,6 +14,8 @@ use std::collections::{HashMap, HashSet}; use std::fs::{self, File, OpenOptions}; use std::io::Read; +use std::os::fd::AsRawFd; +use std::os::unix::fs::OpenOptionsExt; use std::path::{Component, Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -43,10 +45,18 @@ pub struct Message { pub tags: Vec, /// `priority:` — `low` | `normal` | `high`, if set. pub priority: Option, + /// `idempotency-key:` — the optional sender key for local retry deduplication. + pub idempotency_key: Option, /// The markdown body. pub body: String, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum IdempotentSendSeam { + BeforePublication, + AfterPublication, +} + /// Current unix time in milliseconds. pub fn now_ms() -> u64 { SystemTime::now() @@ -107,9 +117,23 @@ pub fn render_message( in_reply_to: Option<&str>, tags: &[String], body: &str, +) -> String { + render_message_fields(from, subject, in_reply_to, tags, body, None) +} + +fn render_message_fields( + from: &str, + subject: Option<&str>, + in_reply_to: Option<&str>, + tags: &[String], + body: &str, + idempotency_key: Option<&str>, ) -> String { let mut s = String::from("---\n"); s.push_str(&format!("from: {from}\n")); + if let Some(key) = idempotency_key { + s.push_str(&format!("idempotency-key: {key}\n")); + } if let Some(subj) = subject { s.push_str(&format!("subject: {subj}\n")); } @@ -142,6 +166,7 @@ fn parse_message(filename: &str, contents: &str) -> Message { in_reply_to: None, tags: Vec::new(), priority: None, + idempotency_key: None, body: String::new(), }; @@ -173,6 +198,7 @@ fn parse_message(filename: &str, contents: &str) -> Message { .collect() } "priority" => msg.priority = Some(v.to_string()), + "idempotency-key" => msg.idempotency_key = Some(v.to_string()), _ => {} } } @@ -200,12 +226,139 @@ pub fn send_to_inbox( ) -> anyhow::Result { fs::create_dir_all(inbox_dir)?; let contents = render_message(from, subject, in_reply_to, tags, body); + publish_message_with_seam(inbox_dir, &contents, |_| Ok(())) +} + +/// Send one normal message with a local idempotency key. +/// +/// While the short local lock is held, st2 searches the recipient inbox first and archive second. +/// A retry returns the first matching normal message. If that message is deleted, st2 forgets the +/// key and a later send creates a new message. +#[allow(clippy::too_many_arguments)] +pub fn send_idempotent_to_inbox( + inbox_dir: &Path, + from: &str, + subject: Option<&str>, + in_reply_to: Option<&str>, + tags: &[String], + body: &str, + idempotency_key: &str, +) -> anyhow::Result { + send_idempotent_to_inbox_with_seam( + inbox_dir, + from, + subject, + in_reply_to, + tags, + body, + idempotency_key, + |_| Ok(()), + ) +} + +#[allow(clippy::too_many_arguments)] +fn send_idempotent_to_inbox_with_seam( + inbox_dir: &Path, + from: &str, + subject: Option<&str>, + in_reply_to: Option<&str>, + tags: &[String], + body: &str, + idempotency_key: &str, + seam: impl FnMut(IdempotentSendSeam) -> anyhow::Result<()>, +) -> anyhow::Result { + validate_idempotency_key(idempotency_key)?; + fs::create_dir_all(inbox_dir)?; + let _lock = MessageIdempotencyLock::acquire(inbox_dir)?; + + if let Some(filename) = find_idempotent_message(inbox_dir, idempotency_key)? { + return Ok(filename); + } + + let contents = render_message_fields( + from, + subject, + in_reply_to, + tags, + body, + Some(idempotency_key), + ); + publish_message_with_seam(inbox_dir, &contents, seam) +} + +fn validate_idempotency_key(value: &str) -> anyhow::Result<()> { + anyhow::ensure!( + !value.is_empty() && value.trim() == value && !value.chars().any(char::is_control), + "message idempotency key must be non-empty single-line text without surrounding whitespace" + ); + Ok(()) +} + +fn find_idempotent_message(inbox_dir: &Path, key: &str) -> anyhow::Result> { + for message in list_dir(inbox_dir)? { + if message.idempotency_key.as_deref() == Some(key) { + return Ok(Some(message.filename)); + } + } + for message in list_dir(&sibling_archive_dir(inbox_dir))? { + if message.idempotency_key.as_deref() == Some(key) { + return Ok(Some(message.filename)); + } + } + Ok(None) +} + +const IDEMPOTENCY_LOCK_FILE: &str = ".message-idempotency.lock"; + +struct MessageIdempotencyLock { + file: File, +} + +impl MessageIdempotencyLock { + fn acquire(inbox_dir: &Path) -> anyhow::Result { + let path = inbox_dir.join(IDEMPOTENCY_LOCK_FILE); + let file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .mode(0o600) + .custom_flags(libc::O_CLOEXEC | libc::O_NOFOLLOW) + .open(&path) + .with_context(|| format!("open message idempotency lock {}", path.display()))?; + anyhow::ensure!( + file.metadata()?.is_file(), + "message idempotency lock is not a regular file: {}", + path.display() + ); + let result = unsafe { libc::flock(file.as_raw_fd(), libc::LOCK_EX) }; + if result != 0 { + return Err(std::io::Error::last_os_error()) + .with_context(|| format!("lock message idempotency file {}", path.display())); + } + Ok(Self { file }) + } +} + +impl Drop for MessageIdempotencyLock { + fn drop(&mut self) { + unsafe { + libc::flock(self.file.as_raw_fd(), libc::LOCK_UN); + } + } +} + +fn publish_message_with_seam( + inbox_dir: &Path, + contents: &str, + mut seam: impl FnMut(IdempotentSendSeam) -> anyhow::Result<()>, +) -> anyhow::Result { // This deliberately cannot match `is_message_filename`, so a concurrent scan ignores it. let tmp = inbox_dir.join(tmp_name()); - if let Err(error) = fs::write(&tmp, &contents) { + if let Err(error) = fs::write(&tmp, contents) { let _ = fs::remove_file(&tmp); return Err(error.into()); } + seam(IdempotentSendSeam::BeforePublication)?; for _ in 0..8 { let filename = new_filename(); let path = inbox_dir.join(&filename); @@ -214,6 +367,7 @@ pub fn send_to_inbox( let _ = fs::remove_file(&tmp); return Err(error.into()); } + seam(IdempotentSendSeam::AfterPublication)?; return Ok(filename); } } @@ -841,6 +995,56 @@ pub fn send_to_resolved_inbox( in_reply_to: Option<&str>, tags: &[String], body: &str, +) -> anyhow::Result { + send_to_resolved_inbox_with_key( + catalog_root, + recipient, + this_host, + from, + subject, + in_reply_to, + tags, + body, + None, + ) +} + +#[allow(clippy::too_many_arguments)] +pub fn send_idempotent_to_resolved_inbox( + catalog_root: &Path, + recipient: &str, + this_host: &str, + from: &str, + subject: Option<&str>, + in_reply_to: Option<&str>, + tags: &[String], + body: &str, + idempotency_key: &str, +) -> anyhow::Result { + send_to_resolved_inbox_with_key( + catalog_root, + recipient, + this_host, + from, + subject, + in_reply_to, + tags, + body, + Some(idempotency_key), + ) +} + +#[allow(clippy::too_many_arguments)] +fn send_to_resolved_inbox_with_key( + catalog_root: &Path, + recipient: &str, + this_host: &str, + from: &str, + subject: Option<&str>, + in_reply_to: Option<&str>, + tags: &[String], + body: &str, + idempotency_key: Option<&str>, ) -> anyhow::Result { let agent = match resolve_agent_handle(catalog_root, recipient, this_host)? { Some(agent) => agent, @@ -851,13 +1055,14 @@ pub fn send_to_resolved_inbox( && discovered.specs.is_empty() && discovered.errors.is_empty() { - return send_to_inbox( + return send_to_inbox_with_optional_key( &catalog_root.join(recipient).join("inbox"), from, subject, in_reply_to, tags, body, + idempotency_key, ); } anyhow::bail!( @@ -870,26 +1075,44 @@ pub fn send_to_resolved_inbox( if let Some(capability) = agent.capability.as_ref() { let inbox = open_message_box(capability, &["resources", "inbox"], true)? .context("created inbox capability is missing")?; - send_to_inbox( + send_to_inbox_with_optional_key( &crate::catalog_transaction::retained_dir_path(&inbox)?, from, subject, in_reply_to, tags, body, + idempotency_key, ) } else { - send_to_inbox( + send_to_inbox_with_optional_key( &inbox_dir(&agent.path), from, subject, in_reply_to, tags, body, + idempotency_key, ) } } +#[allow(clippy::too_many_arguments)] +fn send_to_inbox_with_optional_key( + inbox: &Path, + from: &str, + subject: Option<&str>, + in_reply_to: Option<&str>, + tags: &[String], + body: &str, + idempotency_key: Option<&str>, +) -> anyhow::Result { + match idempotency_key { + Some(key) => send_idempotent_to_inbox(inbox, from, subject, in_reply_to, tags, body, key), + None => send_to_inbox(inbox, from, subject, in_reply_to, tags, body), + } +} + #[cfg(debug_assertions)] fn test_capability_checkpoint() { let (Ok(ready), Ok(release)) = ( @@ -1161,6 +1384,145 @@ mod tests { assert_eq!(fs::read(archive.join(&filename)).unwrap(), receipt); } + fn send_with_key(inbox: &Path, key: &str, body: &str) -> String { + send_idempotent_to_inbox( + inbox, + "producer", + Some("retryable message"), + None, + &[], + body, + key, + ) + .unwrap() + } + + #[test] + fn idempotent_send_follows_inbox_archive_and_deletion_lifetime() { + let temporary = tempfile::tempdir().unwrap(); + let inbox = temporary.path().join("inbox"); + let archive = temporary.path().join("archive"); + + let first = send_with_key(&inbox, "daily-2026-07-31", "first body"); + let first_bytes = fs::read(inbox.join(&first)).unwrap(); + let first_modified = fs::metadata(inbox.join(&first)) + .unwrap() + .modified() + .unwrap(); + let parsed = read_msg(&inbox, &first).unwrap(); + assert_eq!(parsed.idempotency_key.as_deref(), Some("daily-2026-07-31")); + + std::thread::sleep(std::time::Duration::from_millis(5)); + let inbox_retry = send_with_key(&inbox, "daily-2026-07-31", "changed body"); + assert_eq!(inbox_retry, first); + assert_eq!(list_dir(&inbox).unwrap().len(), 1); + assert_eq!(fs::read(inbox.join(&first)).unwrap(), first_bytes); + assert_eq!( + fs::metadata(inbox.join(&first)) + .unwrap() + .modified() + .unwrap(), + first_modified, + "a retry must not rewrite the DING-triggering inbox file" + ); + + archive_msg(&inbox, &archive, &first).unwrap(); + let archived_retry = send_with_key(&inbox, "daily-2026-07-31", "third body"); + assert_eq!(archived_retry, first); + assert!(list_dir(&inbox).unwrap().is_empty()); + assert_eq!(list_dir(&archive).unwrap().len(), 1); + + fs::remove_file(archive.join(&first)).unwrap(); + let after_delete = send_with_key(&inbox, "daily-2026-07-31", "new lifetime"); + assert_ne!(after_delete, first); + assert_eq!( + read_msg(&inbox, &after_delete).unwrap().body.trim_end(), + "new lifetime" + ); + assert!(!temporary.path().join("message-receipts").exists()); + } + + #[test] + fn concurrent_retries_create_one_normal_message() { + use std::sync::{Arc, Barrier}; + + let temporary = tempfile::tempdir().unwrap(); + let inbox = Arc::new(temporary.path().join("inbox")); + let workers = 12; + let barrier = Arc::new(Barrier::new(workers)); + let mut threads = Vec::new(); + for index in 0..workers { + let inbox = Arc::clone(&inbox); + let barrier = Arc::clone(&barrier); + threads.push(std::thread::spawn(move || { + barrier.wait(); + send_with_key(&inbox, "delivery-42", &format!("body {index}")) + })); + } + let filenames: Vec = threads + .into_iter() + .map(|thread| thread.join().unwrap()) + .collect(); + assert!(filenames.iter().all(|filename| filename == &filenames[0])); + assert_eq!(list_dir(&inbox).unwrap().len(), 1); + + let other = send_with_key(&inbox, "delivery-43", "other"); + assert_ne!(other, filenames[0]); + assert_eq!(list_dir(&inbox).unwrap().len(), 2); + } + + #[test] + fn retry_recovers_at_each_message_publication_crash_seam() { + for crash in [ + IdempotentSendSeam::BeforePublication, + IdempotentSendSeam::AfterPublication, + ] { + let temporary = tempfile::tempdir().unwrap(); + let inbox = temporary.path().join("inbox"); + let failed = send_idempotent_to_inbox_with_seam( + &inbox, + "producer", + Some("crash seam"), + None, + &[], + "first body", + "alert-1", + |seam| { + if seam == crash { + anyhow::bail!("injected crash at {seam:?}"); + } + Ok(()) + }, + ); + assert!(failed.is_err(), "{crash:?}"); + + let retry = send_with_key(&inbox, "alert-1", "retry body"); + assert_eq!(list_dir(&inbox).unwrap().len(), 1, "{crash:?}"); + let message = read_msg(&inbox, &retry).unwrap(); + let expected_body = match crash { + IdempotentSendSeam::BeforePublication => "retry body", + IdempotentSendSeam::AfterPublication => "first body", + }; + assert_eq!(message.body.trim_end(), expected_body, "{crash:?}"); + } + } + + #[test] + fn inbox_match_wins_before_archive_match() { + let temporary = tempfile::tempdir().unwrap(); + let inbox = temporary.path().join("inbox"); + let archive = temporary.path().join("archive"); + let archived = send_with_key(&inbox, "same", "archived"); + archive_msg(&inbox, &archive, &archived).unwrap(); + + let contents = render_message_fields("producer", None, None, &[], "inbox", Some("same")); + let inbox_copy = publish_message_with_seam(&inbox, &contents, |_| Ok(())).unwrap(); + assert_ne!(inbox_copy, archived); + + let selected = send_with_key(&inbox, "same", "retry"); + assert_eq!(selected, inbox_copy); + } + #[test] fn resolve_inbox_falls_back_to_the_flat_bus_when_catalog_less() { let tmp = tempfile::tempdir().unwrap(); diff --git a/tests/message_cli.rs b/tests/message_cli.rs index bd4352ba..2bbecb4b 100644 --- a/tests/message_cli.rs +++ b/tests/message_cli.rs @@ -40,6 +40,139 @@ fn list(root: &Path, extra: &[&str]) -> std::process::Output { list_identity(root, "bob", extra) } +fn send(root: &Path, body: &str, extra: &[&str]) -> std::process::Output { + Command::new(env!("CARGO_BIN_EXE_st2")) + .args(["message", "send", "bob", "--root"]) + .arg(root) + .args(["--host", "h", "--as", "alice", "--message", body]) + .args(extra) + .output() + .unwrap() +} + +fn archive(root: &Path, filename: &str) -> std::process::Output { + Command::new(env!("CARGO_BIN_EXE_st2")) + .args(["message", "archive", "bob", filename, "--root"]) + .arg(root) + .args(["--host", "h", "--as", "alice"]) + .output() + .unwrap() +} + +#[test] +fn ordinary_send_keeps_its_bytes_output_and_storage_path() { + let temporary = tempfile::tempdir().unwrap(); + let output = send( + temporary.path(), + "ordinary body", + &["--subject", "ordinary"], + ); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let filename = String::from_utf8(output.stdout).unwrap().trim().to_string(); + assert!(st2::message::is_message_filename(&filename)); + assert_eq!( + fs::read_to_string(temporary.path().join("bob/inbox").join(filename)).unwrap(), + "---\nfrom: alice\nsubject: ordinary\n---\nordinary body\n" + ); + assert!( + !temporary + .path() + .join("bob/inbox/.message-idempotency.lock") + .exists() + ); +} + +#[test] +fn idempotent_send_tracks_the_normal_message_lifetime() { + let temporary = tempfile::tempdir().unwrap(); + let flags = [ + "--subject", + "daily check", + "--idempotency-key", + "daily-2026-07-31", + ]; + let first = send(temporary.path(), "first body", &flags); + assert!( + first.status.success(), + "{}", + String::from_utf8_lossy(&first.stderr) + ); + let filename = String::from_utf8(first.stdout).unwrap().trim().to_string(); + assert!(st2::message::is_message_filename(&filename)); + + let retry = send(temporary.path(), "changed body must not win", &flags); + assert!(retry.status.success()); + assert_eq!(String::from_utf8_lossy(&retry.stdout).trim(), filename); + let message = st2::message::read_msg(&temporary.path().join("bob/inbox"), &filename).unwrap(); + assert_eq!(message.body.trim_end(), "first body"); + assert_eq!(message.idempotency_key.as_deref(), Some("daily-2026-07-31")); + let listed = list(temporary.path(), &["--count"]); + assert!(listed.status.success()); + assert_eq!(String::from_utf8_lossy(&listed.stdout).trim(), "1"); + + let archived = archive(temporary.path(), &filename); + assert!( + archived.status.success(), + "{}", + String::from_utf8_lossy(&archived.stderr) + ); + let retry = send(temporary.path(), "another changed body", &flags); + assert!(retry.status.success()); + assert_eq!(String::from_utf8_lossy(&retry.stdout).trim(), filename); + assert!( + st2::message::list_dir(&temporary.path().join("bob/inbox")) + .unwrap() + .is_empty() + ); + assert_eq!( + st2::message::list_dir(&temporary.path().join("bob/archive")) + .unwrap() + .len(), + 1 + ); + + fs::remove_file(temporary.path().join("bob/archive").join(&filename)).unwrap(); + let after_delete = send(temporary.path(), "new lifetime", &flags); + assert!(after_delete.status.success()); + let after_delete = String::from_utf8(after_delete.stdout) + .unwrap() + .trim() + .to_string(); + assert_ne!(after_delete, filename); + assert_eq!( + st2::message::read_msg(&temporary.path().join("bob/inbox"), &after_delete) + .unwrap() + .body + .trim_end(), + "new lifetime" + ); + assert!(!temporary.path().join("bob/message-receipts").exists()); +} + +#[test] +fn different_keys_create_different_normal_messages_and_invalid_keys_fail() { + let temporary = tempfile::tempdir().unwrap(); + let invalid = send(temporary.path(), "body", &["--idempotency-key", " leading"]); + assert!(!invalid.status.success()); + assert!(!temporary.path().join("bob/inbox").exists()); + + let first = send(temporary.path(), "one", &["--idempotency-key", "one"]); + let second = send(temporary.path(), "two", &["--idempotency-key", "two"]); + assert!(first.status.success()); + assert!(second.status.success()); + assert_ne!(first.stdout, second.stdout); + assert_eq!( + st2::message::list_dir(&temporary.path().join("bob/inbox")) + .unwrap() + .len(), + 2 + ); +} + #[test] fn since_is_strict_and_composes_with_other_list_filters() { let tmp = tempfile::tempdir().unwrap();