mirror of
https://github.com/openai/codex.git
synced 2026-10-14 16:38:43 +00:00
Include attributed previews in agent message board notifications (#47075)
## Why Message board notifications exposed only post and thread IDs, requiring a read to see any post content. ## What changed Deliver notifications as `agent_message` items with author and recipient attribution. Include a `CHANNEL_POST` payload with the sender, channel, message and thread IDs, and up to 150 characters of post text. Direct agents to `read_post` when the preview is truncated, and retain recognition of legacy user-role notices. ## Testing Update the model-context scenario to assert the notification's author, recipient, and preview payload, and update the idle-agent scenario to check for the new notification format. GitOrigin-RevId: e0e2794be82e38bba47f41fb4fc8128eb5e47619
This commit is contained in:
@@ -15,7 +15,7 @@ use codex_agent_message_board_extension::AgentMessageBoard;
|
||||
use codex_agent_message_board_extension::LocalAgentMessageBoard;
|
||||
use codex_agent_message_board_extension::MessageBoardHost;
|
||||
use codex_agent_message_board_extension::NotificationDelivery;
|
||||
use codex_agent_message_board_extension::PostMetadata;
|
||||
use codex_agent_message_board_extension::PostPreview;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::AgentPath;
|
||||
@@ -24,6 +24,7 @@ use codex_protocol::ThreadId;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::CodexErrorDetails;
|
||||
use codex_protocol::error::Result;
|
||||
use codex_protocol::protocol::InterAgentCommunication;
|
||||
use futures::future::BoxFuture;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Weak;
|
||||
@@ -141,14 +142,14 @@ impl MessageBoardHost for LocalBoardHost {
|
||||
|
||||
fn notify(
|
||||
&self,
|
||||
recipient: ThreadId,
|
||||
post: PostMetadata,
|
||||
recipient_id: ThreadId,
|
||||
post: PostPreview,
|
||||
) -> BoxFuture<'_, Result<NotificationDelivery>> {
|
||||
Box::pin(async move {
|
||||
let Some(manager) = self.manager.upgrade() else {
|
||||
return Ok(NotificationDelivery::SkippedInactive);
|
||||
};
|
||||
let recipient = match manager.get_thread(recipient).await {
|
||||
let recipient = match manager.get_thread(recipient_id).await {
|
||||
Ok(thread) => thread,
|
||||
Err(error) if matches!(error.details(), CodexErrorDetails::ThreadNotFound(_)) => {
|
||||
return Ok(NotificationDelivery::SkippedInactive);
|
||||
@@ -160,13 +161,24 @@ impl MessageBoardHost for LocalBoardHost {
|
||||
"notification recipient belongs to another board".into(),
|
||||
));
|
||||
}
|
||||
let notice = AgentMessageBoardNotification {
|
||||
message_id: post.message_id,
|
||||
thread_id: post.thread_id,
|
||||
};
|
||||
let recipient_path = recipient
|
||||
.session
|
||||
.services
|
||||
.agent_control
|
||||
.ensure_agent_known(recipient_id)?
|
||||
.agent_path
|
||||
.ok_or_else(|| CodexErr::InvalidRequest("agent has no tree path".into()))?;
|
||||
let notice = AgentMessageBoardNotification(post);
|
||||
let communication = InterAgentCommunication::new(
|
||||
notice.0.metadata.author.clone(),
|
||||
recipient_path,
|
||||
Vec::new(),
|
||||
notice.render(),
|
||||
/*trigger_turn*/ false,
|
||||
);
|
||||
Ok(
|
||||
match recipient
|
||||
.inject_if_running(vec![ContextualUserFragment::into(notice)])
|
||||
.inject_if_running(vec![communication.to_model_input_item()])
|
||||
.await
|
||||
{
|
||||
Ok(()) => NotificationDelivery::Accepted,
|
||||
|
||||
@@ -1,34 +1,42 @@
|
||||
//! Fixed-size discussion notices. Post text is fetched through bounded tools.
|
||||
//! Attributed discussion notices with bounded previews.
|
||||
|
||||
use super::ContextualUserFragment;
|
||||
use codex_agent_message_board_extension::PostPreview;
|
||||
use codex_protocol::models::ContentItemKind;
|
||||
use uuid::Uuid;
|
||||
|
||||
pub(crate) struct AgentMessageBoardNotification {
|
||||
pub(crate) message_id: Uuid,
|
||||
pub(crate) thread_id: Uuid,
|
||||
}
|
||||
pub(crate) struct AgentMessageBoardNotification(pub(crate) PostPreview);
|
||||
|
||||
impl ContextualUserFragment for AgentMessageBoardNotification {
|
||||
fn role(&self) -> &'static str {
|
||||
"user"
|
||||
"assistant"
|
||||
}
|
||||
fn content_kind(&self) -> ContentItemKind {
|
||||
ContentItemKind("agent_message_board.notification".into())
|
||||
}
|
||||
fn markers(&self) -> (&'static str, &'static str) {
|
||||
Self::type_markers()
|
||||
("", "")
|
||||
}
|
||||
fn type_markers() -> (&'static str, &'static str) {
|
||||
// Keep recognizing user-role notices in rollouts written before this format.
|
||||
(
|
||||
"<agent_message_board_notification>",
|
||||
"</agent_message_board_notification>",
|
||||
)
|
||||
}
|
||||
fn body(&self) -> String {
|
||||
let post = &self.0;
|
||||
let suffix = if post.truncated {
|
||||
"\n[Use read_post for the rest.]"
|
||||
} else {
|
||||
""
|
||||
};
|
||||
format!(
|
||||
"\nNew post {} in discussion {}. Use read_post or read_thread to read it.\n",
|
||||
self.message_id, self.thread_id
|
||||
"Message Type: CHANNEL_POST\nSender: {}\nChannel: {}\nMessage ID: {}\nThread ID: {}\nPayload:\n{}{suffix}",
|
||||
post.metadata.author,
|
||||
post.metadata.channel_name,
|
||||
post.metadata.message_id,
|
||||
post.metadata.thread_id,
|
||||
post.text_preview,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -163,13 +163,32 @@ async fn board_active_notice_and_reads_reach_model_context() -> anyhow::Result<(
|
||||
.expect("search result"),
|
||||
)?;
|
||||
assert_eq!(result["results"][0]["text_preview"], "A shared decision.");
|
||||
let notice = requests[1]
|
||||
.message_input_texts("user")
|
||||
.into_iter()
|
||||
.find(|text| text.contains("<agent_message_board_notification>"))
|
||||
.expect("active notification");
|
||||
assert!(notice.contains(post["message_id"].as_str().expect("post id")));
|
||||
assert!(!notice.contains("A shared decision."));
|
||||
let input = requests[1].body_json();
|
||||
let notices = input["input"]
|
||||
.as_array()
|
||||
.context("request input")?
|
||||
.iter()
|
||||
.filter(|item| item["type"] == "agent_message")
|
||||
.map(|item| {
|
||||
(
|
||||
item["author"].clone(),
|
||||
item["recipient"].clone(),
|
||||
item["content"].clone(),
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let message_id = post["message_id"].as_str().context("message ID")?;
|
||||
assert_eq!(
|
||||
notices,
|
||||
vec![(
|
||||
json!("/root"),
|
||||
json!("/root"),
|
||||
json!([{
|
||||
"type": "input_text",
|
||||
"text": format!("Message Type: CHANNEL_POST\nSender: /root\nChannel: design\nMessage ID: {message_id}\nThread ID: {message_id}\nPayload:\nA shared decision."),
|
||||
}])
|
||||
)]
|
||||
);
|
||||
// Cargo and Bazel can enable different serde_json ordering features. Normalize only
|
||||
// the snapshot copies so the assertion compares JSON content, not object key order.
|
||||
let mut bodies = requests
|
||||
@@ -189,7 +208,7 @@ async fn board_active_notice_and_reads_reach_model_context() -> anyhow::Result<(
|
||||
insta::assert_snapshot!(
|
||||
"agent_message_board_context",
|
||||
context_snapshot::format_context_snapshot(
|
||||
"An active agent posts a shared decision, receives an ID-only notice, and fetches the text.",
|
||||
"An active agent posts a shared decision, receives an attributed preview, and fetches the text.",
|
||||
&bodies.iter().map(SnapshotEntry::body).collect::<Vec<_>>(),
|
||||
&ContextSnapshotOptions::default().rewrite_known_segments(),
|
||||
)
|
||||
@@ -287,7 +306,7 @@ async fn board_is_shared_with_children_survives_resume_and_skips_idle_notices()
|
||||
assert!(
|
||||
read.requests()
|
||||
.iter()
|
||||
.all(|request| !request.body_contains_text("<agent_message_board_notification>"))
|
||||
.all(|request| !request.body_contains_text("Message Type: CHANNEL_POST"))
|
||||
);
|
||||
let output = read.requests()[1]
|
||||
.function_call_output_text("root-read")
|
||||
|
||||
+3
-6
@@ -1,9 +1,9 @@
|
||||
---
|
||||
source: core/tests/suite/scenarios_agent_message_board.rs
|
||||
assertion_line: 88
|
||||
expression: "context_snapshot::format_context_snapshot(\"An active agent posts a shared decision, receives an ID-only notice, and fetches the text.\",\n&bodies.iter().map(SnapshotEntry::body).collect::<Vec<_>>(), &ContextSnapshotOptions::default().rewrite_known_segments(),)"
|
||||
expression: "context_snapshot::format_context_snapshot(\"An active agent posts a shared decision, receives an attributed preview, and fetches the text.\",\n&bodies.iter().map(SnapshotEntry::body).collect::<Vec<_>>(), &ContextSnapshotOptions::default().rewrite_known_segments(),)"
|
||||
---
|
||||
Scenario: An active agent posts a shared decision, receives an ID-only notice, and fetches the text.
|
||||
Scenario: An active agent posts a shared decision, receives an attributed preview, and fetches the text.
|
||||
|
||||
## Window 1
|
||||
-- request 1 (request) --
|
||||
@@ -36,10 +36,7 @@ Scenario: An active agent posts a shared decision, receives an ID-only notice, a
|
||||
-- request 2 (request) --
|
||||
05:function_call/collaboration.post:{"agents_to_notify":["/root"],"new_channel_name":"design","text":"A shared decision."}
|
||||
06:function_call_output:{"author":"/root","channel_name":"design","created_at":"2026-09-18T12:00:00Z","message_id":"<UUID 1>","thread_id":"<UUID 1>"}
|
||||
07:message/user:
|
||||
<agent_message_board_notification>
|
||||
New post <UUID 1> in discussion <UUID 1>. Use read_post or read_thread to read it.
|
||||
</agent_message_board_notification>
|
||||
07:agent_message
|
||||
-- request 3 (request) --
|
||||
08:function_call/collaboration.search_posts:{"channel_name":"design","query":"decision"}
|
||||
09:function_call_output:{"has_more":false,"n_returned":1,"next_cursor":null,"results":[{"author":"/root","channel_name":"design"...ecision.","thread_id":"<UUID 1>","truncated":false}]} [hash=facdb1e4f66cc5b9]
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
//! Host capabilities needed by a board without depending on codex-core.
|
||||
|
||||
use crate::PostMetadata;
|
||||
use crate::PostPreview;
|
||||
use chrono::DateTime;
|
||||
use chrono::Utc;
|
||||
use codex_protocol::AgentPath;
|
||||
@@ -28,10 +28,10 @@ pub trait MessageBoardHost: Send + Sync {
|
||||
|
||||
fn current_time(&self, caller: ThreadId) -> BoxFuture<'_, Result<DateTime<Utc>>>;
|
||||
|
||||
/// Only metadata is pushed. Full content is retrieved through bounded reads.
|
||||
/// Push metadata and a bounded preview. Full content is retrieved through bounded reads.
|
||||
fn notify(
|
||||
&self,
|
||||
recipient: ThreadId,
|
||||
post: PostMetadata,
|
||||
post: PostPreview,
|
||||
) -> BoxFuture<'_, Result<NotificationDelivery>>;
|
||||
}
|
||||
|
||||
@@ -283,11 +283,12 @@ impl LocalAgentMessageBoard {
|
||||
tx.commit().await.map_err(storage_error)?;
|
||||
|
||||
// A committed post succeeds even if a best-effort notice cannot be delivered.
|
||||
let notice = paging::preview(post.clone(), /*max_chars*/ 150);
|
||||
futures::stream::iter(recipients)
|
||||
.for_each_concurrent(/*limit*/ 16, |recipient| {
|
||||
let metadata = post.metadata.clone();
|
||||
let notice = notice.clone();
|
||||
async move {
|
||||
if let Err(error) = self.host.notify(recipient, metadata).await {
|
||||
if let Err(error) = self.host.notify(recipient, notice).await {
|
||||
tracing::warn!(%recipient, %error, "Failed to deliver message-board notification");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
use super::*;
|
||||
use crate::NotificationDelivery;
|
||||
use crate::PostPreview;
|
||||
use futures::future::BoxFuture;
|
||||
use pretty_assertions::assert_ne;
|
||||
|
||||
@@ -23,7 +24,7 @@ impl MessageBoardHost for UnusedHost {
|
||||
fn notify(
|
||||
&self,
|
||||
_recipient: ThreadId,
|
||||
_post: PostMetadata,
|
||||
_post: PostPreview,
|
||||
) -> BoxFuture<'_, Result<NotificationDelivery>> {
|
||||
unreachable!("pool tests do not invoke host capabilities")
|
||||
}
|
||||
|
||||
@@ -62,7 +62,7 @@ impl MessageBoardHost for Host {
|
||||
fn notify(
|
||||
&self,
|
||||
recipient: ThreadId,
|
||||
post: PostMetadata,
|
||||
post: PostPreview,
|
||||
) -> BoxFuture<'_, Result<NotificationDelivery>> {
|
||||
Box::pin(async move {
|
||||
if self.fail_notifications.load(Ordering::SeqCst) {
|
||||
@@ -76,7 +76,7 @@ impl MessageBoardHost for Host {
|
||||
self.notifications
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.push((recipient, post));
|
||||
.push((recipient, post.metadata));
|
||||
Ok(NotificationDelivery::Accepted)
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user