recv: drop wait_seconds from the MCP tool, always an immediate peek
This commit is contained in:
parent
9e7158f593
commit
ceff662d25
6 changed files with 59 additions and 117 deletions
|
|
@ -21,20 +21,12 @@ pub struct SendArgs {
|
|||
|
||||
#[derive(Debug, serde::Deserialize, schemars::JsonSchema)]
|
||||
pub struct RecvArgs {
|
||||
/// How long to long-poll for the FIRST message before returning
|
||||
/// the empty marker. Capped at 60s server-side. Default (None)
|
||||
/// is 30s. Useful when an agent wants to park its turn waiting
|
||||
/// for any new work — pick a longer wait to coalesce bursts.
|
||||
#[serde(default)]
|
||||
pub wait_seconds: Option<u64>,
|
||||
/// Maximum number of messages to pop in this round-trip. Default
|
||||
/// (None) is 1 (single-message behaviour — exactly what you want
|
||||
/// when you're called to drive a turn off the first wake). Pass
|
||||
/// a higher value (capped at 5 server-side) when you've been
|
||||
/// told the inbox has more queued (the wake prompt mentions
|
||||
/// pending count) and want to drain everything in one tool call.
|
||||
/// Once the long-poll wakes up, the call drains up to `max` in
|
||||
/// total before returning — no extra round-trip needed.
|
||||
#[serde(default)]
|
||||
pub max: Option<u32>,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -276,24 +276,21 @@ impl AgentServer {
|
|||
|
||||
#[tool(
|
||||
description = "Pop messages from this agent's inbox. Returns one or more messages, or \
|
||||
an empty marker if nothing is waiting. \n\n\
|
||||
an empty marker if nothing is waiting. Always an immediate 'anything pending?' peek \
|
||||
— never blocks. \n\n\
|
||||
**Single-message default**: with no args (or `max: 1`) you get the next message — \
|
||||
same behaviour the harness uses to drive a turn. Without `wait_seconds` (or with 0) \
|
||||
the call returns immediately — a cheap 'anything pending?' peek. \n\n\
|
||||
**When idle, prefer ending the turn over parking here.** Ending the turn is the \
|
||||
same behaviour the harness uses to drive a turn. \n\n\
|
||||
**When idle, prefer ending the turn over polling here.** Ending the turn is the \
|
||||
ONLY path that observes an in-container todo wake (bash-task completions, matrix \
|
||||
unread, forge activity) — that signal reaches the harness loop between turns, never \
|
||||
a live `recv` call, so parking in `wait_seconds` while a todo lands means sitting \
|
||||
blind until the timeout. Ending the turn also checkpoints your session, and costs no \
|
||||
latency vs. parking for a real inbox message either way — the broker wakes the next \
|
||||
turn just as fast. Reach for a positive `wait_seconds` (capped at 180) only for a \
|
||||
short, deliberate in-turn block — e.g. confirming something you just triggered lands \
|
||||
within seconds — not as the default way to wait for more work. \n\n\
|
||||
a live `recv` call, so looping on `recv` while idle means sitting blind until \
|
||||
something else wakes you. Ending the turn also checkpoints your session, and costs \
|
||||
no latency vs. polling for a real inbox message either way — the broker wakes the \
|
||||
next turn just as fast. \n\n\
|
||||
**Batch drain**: pass `max: N` (capped at 5) to drain up to N messages in one \
|
||||
round-trip. Use this when the wake prompt told you the inbox has more queued, or \
|
||||
any time you expect a burst — one tool call beats N consecutive single recvs. \
|
||||
`wait_seconds` still applies to the FIRST message; once one arrives the call drains \
|
||||
up to `max` in total. Empty result reported the same way regardless of `max`. \n\n\
|
||||
any time you expect a burst — one tool call beats N consecutive single recvs. Empty \
|
||||
result reported the same way regardless of `max`. \n\n\
|
||||
After popping, the result appends a `(N more message(s) pending …)` line whenever the \
|
||||
inbox still has queued messages — so you know whether to drain again (or `ack_until`) \
|
||||
without a separate status check. No line means the inbox is empty."
|
||||
|
|
@ -301,14 +298,13 @@ impl AgentServer {
|
|||
async fn recv(&self, Parameters(args): Parameters<RecvArgs>) -> String {
|
||||
let log = format!("{args:?}");
|
||||
run_tool_envelope("recv", log, async move {
|
||||
let waited = args.wait_seconds.is_some_and(|w| w > 0);
|
||||
let (resp, retries) = self
|
||||
.dispatch(hive_core_agent_sock::Request::Recv {
|
||||
wait_seconds: args.wait_seconds,
|
||||
wait_seconds: None,
|
||||
max: args.max,
|
||||
})
|
||||
.await;
|
||||
annotate_retries(format_recv(resp, waited), retries)
|
||||
annotate_retries(format_recv(resp), retries)
|
||||
})
|
||||
.await
|
||||
}
|
||||
|
|
|
|||
|
|
@ -39,33 +39,26 @@ pub fn format_ack(
|
|||
|
||||
/// Format helper for `recv`: renders zero, one, or many popped
|
||||
/// messages. Empty list collapses to "(empty)" so claude doesn't go
|
||||
/// hunting for content; when `waited` is set (the call parked on a
|
||||
/// long-poll that timed out) the empty result also carries
|
||||
/// [`IDLE_WAIT_HINT`] nudging the model toward other work. A single
|
||||
/// message renders as the historical `from: X\n\nbody` block (banner
|
||||
/// first if `redelivered`). A multi-message batch renders with a
|
||||
/// `popped N message(s):` header and `---` separators between bodies
|
||||
/// so the model can tell where one ends and the next begins;
|
||||
/// per-message redelivery banners included.
|
||||
/// hunting for content. A single message renders as the historical
|
||||
/// `from: X\n\nbody` block (banner first if `redelivered`). A
|
||||
/// multi-message batch renders with a `popped N message(s):` header
|
||||
/// and `---` separators between bodies so the model can tell where
|
||||
/// one ends and the next begins; per-message redelivery banners
|
||||
/// included.
|
||||
#[must_use]
|
||||
pub fn format_recv(
|
||||
resp: Result<hive_core_agent_sock::Response, anyhow::Error>,
|
||||
waited: bool,
|
||||
) -> String {
|
||||
pub fn format_recv(resp: Result<hive_core_agent_sock::Response, anyhow::Error>) -> String {
|
||||
match resp {
|
||||
Ok(hive_core_agent_sock::Response::Messages {
|
||||
messages,
|
||||
remaining,
|
||||
}) => render_recv_messages(&messages, remaining, waited),
|
||||
}) => render_recv_messages(&messages, remaining),
|
||||
// A graceful stop is pending — the inbox is fenced. Render a single
|
||||
// explicit directive (not an empty inbox, which claude's "park on recv"
|
||||
// habit would long-poll again, stalling the stop-checkpoint turn until
|
||||
// the drain wait times out into a hard stop) so every recv during the
|
||||
// stop unmissably tells claude to flush + end. `remaining` is forced
|
||||
// to 0 — the inbox is fenced, so a "N more pending" hint would be
|
||||
// misleading.
|
||||
// explicit directive (not an empty inbox, which claude might just
|
||||
// recv again) so every recv during the stop unmissably tells claude
|
||||
// to flush + end. `remaining` is forced to 0 — the inbox is fenced,
|
||||
// so a "N more pending" hint would be misleading.
|
||||
Ok(hive_core_agent_sock::Response::GracefulStop) => {
|
||||
render_recv_messages(&[graceful_stop_message()], 0, waited)
|
||||
render_recv_messages(&[graceful_stop_message()], 0)
|
||||
}
|
||||
other => reply_err(other, "recv"),
|
||||
}
|
||||
|
|
@ -92,18 +85,10 @@ fn graceful_stop_message() -> hive_sh4re::DeliveredMessage {
|
|||
/// depth; when non-zero a shared "(N more pending …)" hint (identical to the
|
||||
/// wake prompt's) is appended so an in-turn drain knows more is queued. The
|
||||
/// empty path never carries the hint (nothing was popped).
|
||||
fn render_recv_messages(
|
||||
messages: &[hive_sh4re::DeliveredMessage],
|
||||
remaining: u64,
|
||||
waited: bool,
|
||||
) -> String {
|
||||
fn render_recv_messages(messages: &[hive_sh4re::DeliveredMessage], remaining: u64) -> String {
|
||||
use std::fmt::Write as _;
|
||||
if messages.is_empty() {
|
||||
return if waited {
|
||||
format!("(empty){IDLE_WAIT_HINT}")
|
||||
} else {
|
||||
"(empty)".to_owned()
|
||||
};
|
||||
return "(empty)".to_owned();
|
||||
}
|
||||
let mut out = if messages.len() == 1 {
|
||||
let m = &messages[0];
|
||||
|
|
@ -151,14 +136,6 @@ fn msg_id_tag(id: i64) -> String {
|
|||
}
|
||||
}
|
||||
|
||||
/// Appended to the `recv` empty result when the agent parked on a
|
||||
/// long-poll (`wait_seconds > 0`) that timed out with nothing new.
|
||||
/// Nudges the model to spend the idle time on other useful work
|
||||
/// instead of immediately re-blocking on `recv`.
|
||||
pub const IDLE_WAIT_HINT: &str = " — nothing arrived before the wait timed out. \
|
||||
If you have other useful work (assigned issues, in-flight PRs, a docs sweep, \
|
||||
notes to update), do that now rather than immediately parking on recv again.";
|
||||
|
||||
/// Inner renderer for a `Vec<LooseEnd>` already extracted from the socket
|
||||
/// reply. Called by the `get_loose_ends` handler, which injects the
|
||||
/// `UnreadMatrix` entry before formatting.
|
||||
|
|
@ -544,7 +521,7 @@ pub fn annotate_retries(mut s: String, retries: u32) -> String {
|
|||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{IDLE_WAIT_HINT, format_recv};
|
||||
use super::format_recv;
|
||||
|
||||
fn msg(id: i64, from: &str, body: &str) -> hive_sh4re::DeliveredMessage {
|
||||
hive_sh4re::DeliveredMessage {
|
||||
|
|
@ -557,39 +534,20 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn empty_recv_after_wait_appends_idle_hint() {
|
||||
let out = format_recv(
|
||||
Ok(hive_core_agent_sock::Response::Messages {
|
||||
messages: vec![],
|
||||
remaining: 0,
|
||||
}),
|
||||
true,
|
||||
);
|
||||
assert!(out.starts_with("(empty)"));
|
||||
assert!(out.contains(IDLE_WAIT_HINT));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_recv_without_wait_has_no_hint() {
|
||||
let out = format_recv(
|
||||
Ok(hive_core_agent_sock::Response::Messages {
|
||||
messages: vec![],
|
||||
remaining: 0,
|
||||
}),
|
||||
false,
|
||||
);
|
||||
fn empty_recv_renders_bare_empty_marker() {
|
||||
let out = format_recv(Ok(hive_core_agent_sock::Response::Messages {
|
||||
messages: vec![],
|
||||
remaining: 0,
|
||||
}));
|
||||
assert_eq!(out, "(empty)");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn single_recv_with_remaining_appends_pending_hint() {
|
||||
let out = format_recv(
|
||||
Ok(hive_core_agent_sock::Response::Messages {
|
||||
messages: vec![msg(7, "alice", "hi")],
|
||||
remaining: 3,
|
||||
}),
|
||||
false,
|
||||
);
|
||||
let out = format_recv(Ok(hive_core_agent_sock::Response::Messages {
|
||||
messages: vec![msg(7, "alice", "hi")],
|
||||
remaining: 3,
|
||||
}));
|
||||
assert!(out.starts_with("[msg #7] from: alice"));
|
||||
assert!(out.contains("3 more message(s) pending"));
|
||||
assert!(out.contains("max: 3"));
|
||||
|
|
@ -597,25 +555,19 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn single_recv_no_remaining_has_no_pending_hint() {
|
||||
let out = format_recv(
|
||||
Ok(hive_core_agent_sock::Response::Messages {
|
||||
messages: vec![msg(7, "alice", "hi")],
|
||||
remaining: 0,
|
||||
}),
|
||||
false,
|
||||
);
|
||||
let out = format_recv(Ok(hive_core_agent_sock::Response::Messages {
|
||||
messages: vec![msg(7, "alice", "hi")],
|
||||
remaining: 0,
|
||||
}));
|
||||
assert!(!out.contains("more message(s) pending"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn batch_recv_with_remaining_appends_pending_hint_once() {
|
||||
let out = format_recv(
|
||||
Ok(hive_core_agent_sock::Response::Messages {
|
||||
messages: vec![msg(7, "alice", "hi"), msg(8, "bob", "yo")],
|
||||
remaining: 9,
|
||||
}),
|
||||
false,
|
||||
);
|
||||
let out = format_recv(Ok(hive_core_agent_sock::Response::Messages {
|
||||
messages: vec![msg(7, "alice", "hi"), msg(8, "bob", "yo")],
|
||||
remaining: 9,
|
||||
}));
|
||||
assert!(out.starts_with("popped 2 message(s):"));
|
||||
assert_eq!(out.matches("more message(s) pending").count(), 1);
|
||||
// `max` suggestion is clamped to the server-side recv cap.
|
||||
|
|
|
|||
Loading…
Reference in a new issue