Skip to content

Commit ebc3de1

Browse files
committed
fix(approvals): save a request and wait on it before the chat shows it
approvals::open announced the approval's chat item before saving the approval, and the answer's channel was registered after that. A fast app could answer a request that was not in the store yet (not found, seen on CI's Linux runner in a browser test), or answer before the daemon waited on it, so the bot would only hear back when the request expired. Now the chat item and the approval are saved, the answer is waited on, and only then does the app hear of it.
1 parent e4362f9 commit ebc3de1

3 files changed

Lines changed: 95 additions & 16 deletions

File tree

‎crates/botloftd/src/approvals/mod.rs‎

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use botloft_core::chat::{
1515
use botloft_core::command::tool_explanation;
1616
use botloft_core::ids::{ApprovalId, BotId, ChatItemId};
1717
use botloft_core::protocol::{
18-
AllowScope, Approval, ApprovalItem, ApprovalStatus, ApprovalsAnswerParams, ChatBody,
18+
AllowScope, Approval, ApprovalItem, ApprovalStatus, ApprovalsAnswerParams, ChatBody, ChatItem,
1919
};
2020
use botloft_store::ApprovalRecord;
2121
use serde_json::Value;
@@ -103,12 +103,7 @@ pub(crate) async fn ask_with(
103103
tool_use_id: &str,
104104
opened: impl FnOnce(&ApprovalId),
105105
) -> Option<Answer> {
106-
let pending = open(daemon, bot, tool_name, input, tool_use_id)?;
107-
let (answer, waiting) = oneshot::channel();
108-
daemon
109-
.approvals
110-
.lock()
111-
.insert(pending.record.approval.id.clone(), answer);
106+
let (pending, waiting) = open(daemon, bot, tool_name, input, tool_use_id)?;
112107
opened(&pending.record.approval.id);
113108
daemon.supervisor.approval_opened(bot, generation);
114109
let mut guard = Guard {
@@ -220,14 +215,15 @@ struct Pending {
220215
record: ApprovalRecord,
221216
}
222217

223-
/// Saves the request and puts it in the chat.
218+
/// Saves the request, waits for its answer and puts it in the chat, in
219+
/// that order: the owner may answer as soon as the chat shows it.
224220
fn open(
225221
daemon: &Daemon,
226222
bot: &BotId,
227223
tool_name: &str,
228224
input: &Value,
229225
tool_use_id: &str,
230-
) -> Option<Pending> {
226+
) -> Option<(Pending, oneshot::Receiver<Decision>)> {
231227
let now = daemon.clock.now_ms();
232228
let approval = Approval {
233229
id: ApprovalId::generate(),
@@ -245,17 +241,35 @@ fn open(
245241
tool_explanation(tool_name, input),
246242
always::scope_of(tool_name, input),
247243
);
248-
let item = items::add(daemon, bot, ChatBody::Approval(shown))?;
244+
let item = ChatItem {
245+
id: ChatItemId::generate(),
246+
bot_id: bot.clone(),
247+
body: ChatBody::Approval(shown),
248+
created_at: now,
249+
updated_at: now,
250+
};
249251
let record = ApprovalRecord {
250252
approval,
251-
chat_item_id: item.id,
253+
chat_item_id: item.id.clone(),
252254
tool_use_id: tool_use_id.to_owned(),
253255
};
254-
if let Err(err) = daemon.store().insert_approval(&record) {
255-
warn!(bot = %bot, "could not save an approval: {err}");
256-
return None;
256+
{
257+
let store = daemon.store();
258+
let saved = store
259+
.insert_chat_item(&item)
260+
.and_then(|()| store.insert_approval(&record));
261+
if let Err(err) = saved {
262+
warn!(bot = %bot, "could not save an approval: {err}");
263+
return None;
264+
}
257265
}
258-
Some(Pending { record })
266+
let (answer, waiting) = oneshot::channel();
267+
daemon
268+
.approvals
269+
.lock()
270+
.insert(record.approval.id.clone(), answer);
271+
items::announce(daemon, item);
272+
Some((Pending { record }, waiting))
259273
}
260274

261275
fn item_of(
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
//! A request for the owner is saved and waited on before the chat shows it
2+
//! (spec 10.1): an answer sent the moment it shows always lands.
3+
4+
mod common;
5+
6+
use botloft_core::protocol::{ApprovalsAnswerParams, BotState, ChatBody};
7+
use botloftd::state::Event;
8+
use common::TestDaemon;
9+
use common::mcp::Mcp;
10+
use serde_json::json;
11+
12+
#[tokio::test(flavor = "multi_thread")]
13+
async fn an_answer_the_moment_the_request_shows_lands() {
14+
let t = TestDaemon::start_supervised().await;
15+
let mut app = t.session().await;
16+
let lead = json!({ "name": "Chefe", "role": "Leads", "instructions": "Build the site" });
17+
let crew = app
18+
.call("crews.create", json!({ "name": "Site", "lead": lead }))
19+
.await
20+
.expect("crew");
21+
let bots = app
22+
.call("bots.list", json!({ "crewId": crew["id"] }))
23+
.await
24+
.expect("bots");
25+
let chief = bots[0].clone();
26+
let process = t.process_of(&chief).await;
27+
t.until_state(&chief, BotState::Idle).await;
28+
let mut mcp = Mcp::new(t.addr, process.env("BOTLOFT_BOT_TOKEN").expect("token"));
29+
30+
for round in 0..20 {
31+
let mut events = t.daemon.subscribe();
32+
let suggestion = json!({
33+
"name": format!("Designer {round}"),
34+
"role": "Draws each page",
35+
"instructions": "Design the pages.",
36+
"reason": "The site needs a look.",
37+
});
38+
let suggesting = tokio::spawn({
39+
let mut mcp = Mcp::new(mcp.addr, mcp.token.clone());
40+
async move { mcp.tool("suggest_bot", suggestion).await }
41+
});
42+
let approval_id = loop {
43+
if let Ok(Event::ChatItem(changed)) = events.recv().await
44+
&& let ChatBody::Approval(item) = changed.item.body
45+
{
46+
break item.approval_id;
47+
}
48+
};
49+
// No wait at all between seeing it and answering.
50+
let answer: ApprovalsAnswerParams =
51+
serde_json::from_value(json!({ "approvalId": approval_id, "allow": false }))
52+
.expect("params");
53+
botloftd::approvals::answer(&t.daemon, answer).expect("the request is there");
54+
// And the bot hears it at once, not when the request runs out.
55+
let refused = tokio::time::timeout(std::time::Duration::from_secs(5), suggesting)
56+
.await
57+
.expect("the bot heard the answer")
58+
.expect("task")
59+
.expect("tool");
60+
assert_eq!(refused["created"], false, "{refused}");
61+
}
62+
mcp.tool("crew_roster", json!({}))
63+
.await
64+
.expect("still fine");
65+
}

‎docs/spec.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ Endereçamento só dentro da crew. Bot não enxerga bots nem tasks de outras cre
576576

577577
Com `--permission-prompt-tool mcp__botloft__permission_prompt`, toda ferramenta que precisaria de permissão chama a tool `permission_prompt`. A entrada é `{tool_name, input, tool_use_id}` (documentado; visto com 2.1.284).
578578

579-
1. O daemon grava uma `approval` (`pending`) e o item `approval` no chat, e passa o bot para `needs_approval`.
579+
1. O daemon grava uma `approval` (`pending`) e o item `approval` no chat, e passa o bot para `needs_approval`. A ordem importa: grava o item e o pedido, passa a esperar a resposta e só então avisa o app (`chat.item`). Avisando antes, um app rápido respondia a um pedido que ainda não estava no banco (`not found`, visto no CI do Linux), ou a resposta chegava antes da espera e o bot só a ouvia quando o pedido expirava.
580580
2. A chamada HTTP fica aberta até o dono responder (`approvals.answer {approvalId, allow, note?}`) ou até `approval_timeout_minutes`, que o dono escolhe nas Configurações (15 min a 8 h no app; o daemon aceita de 1 min a 1 dia). O `timeout` do `mcp.json` (10) só é lido quando o Claude Code inicia: aumentada a espera, cada bot reinicia quando nada estiver em andamento; diminuída, o daemon responde antes e nada reinicia.
581581
3. Resposta ao Claude Code, em texto JSON:
582582
- Permitir: `{"behavior":"allow","updatedInput":<input original>}`.

0 commit comments

Comments
 (0)