diff --git a/apps/ai-providers/host-service/src/complete/mod.rs b/apps/ai-providers/host-service/src/complete/mod.rs index 8d0fe695..67fa0d99 100644 --- a/apps/ai-providers/host-service/src/complete/mod.rs +++ b/apps/ai-providers/host-service/src/complete/mod.rs @@ -687,6 +687,13 @@ impl HostService for ModelService { FAMILY } + /// Every attempt on one provider, and a margin: the app hears a timeout + /// only once no single provider could still answer. A call that falls + /// back across several slow providers can outlast it. + fn timeout(&self, _call: &ServiceCall) -> Duration { + TIMEOUT * ATTEMPTS + Duration::from_secs(30) + } + fn call(&mut self, call: ServiceCall, reply: Replier, _host: &mut dyn ServiceHost) { if !(self.grants)(&call.app_id, &call.host_dir) { return reply.send(Err(Refusal::new(Code::Capability, "This app was not granted the model capability.").to_string())); @@ -722,3 +729,22 @@ pub fn register_with(options: Options) -> Arc { octosense_appstore::services::register_host_service(Box::new(ModelService { host: host.clone(), grants })); host } + +#[cfg(test)] +mod service_timeout { + use super::*; + + #[test] + fn a_model_call_may_wait_out_every_attempt_on_a_provider() { + let service = ModelService { host: Arc::new(ModelHost::new(&Options::default())), grants: Arc::new(|_, _| true) }; + let call = ServiceCall { + app_id: "os.test".into(), + service: "model.complete".into(), + args: serde_json::Value::Null, + from_sheet: false, + may_prompt: true, + host_dir: std::env::temp_dir(), + }; + assert!(service.timeout(&call) >= TIMEOUT * ATTEMPTS, "the host must not time out a call its provider is still allowed to answer"); + } +} diff --git a/apps/ai-providers/host-service/src/lib.rs b/apps/ai-providers/host-service/src/lib.rs index 92f90fe6..e10455d4 100644 --- a/apps/ai-providers/host-service/src/lib.rs +++ b/apps/ai-providers/host-service/src/lib.rs @@ -555,8 +555,14 @@ fn scan_error(reason: &str) -> String { } impl LlmService { - /// Raise `sheet` for `app_id`, replacing any sheet already waiting. - fn raise(&mut self, app_id: &str, reply: Replier, kind: Kind, sheet: String, host: &mut dyn ServiceHost) { + /// Raise `sheet` for `app_id`, replacing any sheet already waiting. A + /// surface that cannot show a sheet (a home-screen tile, an agent's tool + /// call) is refused instead, and a sheet already up stays. Whether it rose. + fn raise(&mut self, app_id: &str, may_prompt: bool, reply: Replier, kind: Kind, sheet: String, host: &mut dyn ServiceHost) -> bool { + if !may_prompt { + reply.send(Err("Open AI providers to do this: it needs its sheet.".into())); + return false; + } if let Some(earlier) = self.pending.lock().unwrap().take() { earlier.reply.send(Err("Another sheet replaced this one.".into())); } @@ -564,6 +570,7 @@ impl LlmService { IMAGE_WAITER.lock().unwrap().take(); *self.pending.lock().unwrap() = Some(Pending { app_id: app_id.to_string(), reply, kind }); host.open_sheet(sheet); + true } fn pending_kind(&self) -> Option { @@ -738,7 +745,7 @@ impl LlmService { }); } - fn export(&mut self, app_id: &str, args: &Value, reply: Replier, host: &mut dyn ServiceHost) { + fn export(&mut self, app_id: &str, may_prompt: bool, args: &Value, reply: Replier, host: &mut dyn ServiceHost) { let store = match self.shared.open() { Ok(s) => s, Err(e) => return reply.send(Err(e)), @@ -755,7 +762,9 @@ impl LlmService { } self.generation += 1; let generation = self.generation; - self.raise(app_id, reply, Kind::Export(generation), sheets::export_waiting(), host); + if !self.raise(app_id, may_prompt, reply, Kind::Export(generation), sheets::export_waiting(), host) { + return; + } *self.export.lock().unwrap() = Some((generation, None)); let (shared, export, pending, lifetime) = (self.shared.clone(), self.export.clone(), self.pending.clone(), self.qr_lifetime); work(move || { @@ -1064,7 +1073,7 @@ impl HostService for LlmService { match call.method() { "connect_client" => { if call.app_id != "os.ai-providers" { return reply.send(Err("Open AI providers to connect a client.".into())); } - self.raise(&call.app_id, reply, Kind::Connect, sheets::connect_client(), host); + self.raise(&call.app_id, call.may_prompt, reply, Kind::Connect, sheets::connect_client(), host); } method if method.starts_with("sheet.client_") => { // Only the Talk to Octos sheet this app raised, never the app. @@ -1113,12 +1122,14 @@ impl HostService for LlmService { "add_provider" => match self.shared.open() { Ok(store) => { let sheet = sheets::edit(None, !store.list.is_empty()); - self.raise(&call.app_id, reply, Kind::Add, sheet, host) + self.raise(&call.app_id, call.may_prompt, reply, Kind::Add, sheet, host); } Err(e) => reply.send(Err(e)), }, "edit_provider" => match self.provider(&id) { - Ok(p) => self.raise(&call.app_id, reply, Kind::Edit(id), sheets::edit(Some(&p), true), host), + Ok(p) => { + self.raise(&call.app_id, call.may_prompt, reply, Kind::Edit(id), sheets::edit(Some(&p), true), host); + } Err(e) => reply.send(Err(e)), }, "set_model" | "move" | "set_primary" | "remove" => { @@ -1163,10 +1174,10 @@ impl HostService for LlmService { reply.send(answer); }); } - "export_qr" => self.export(&call.app_id, &call.args, reply, host), + "export_qr" => self.export(&call.app_id, call.may_prompt, &call.args, reply, host), "import_qr" => { let sheet = sheets::import(self.scanner.is_some(), self.image_picker.is_some(), self.image_drops); - self.raise(&call.app_id, reply, Kind::Import { scanned: None }, sheet, host) + self.raise(&call.app_id, call.may_prompt, reply, Kind::Import { scanned: None }, sheet, host); } "sheet.cancel" => { host.close_sheet(); diff --git a/apps/ai-providers/host-service/tests/service.rs b/apps/ai-providers/host-service/tests/service.rs index 3be8c13d..f88e72df 100644 --- a/apps/ai-providers/host-service/tests/service.rs +++ b/apps/ai-providers/host-service/tests/service.rs @@ -76,6 +76,15 @@ impl Rig { heap } + /// A call from a surface that cannot show a sheet: a home-screen tile, + /// an agent's tool call. + fn send_background(&mut self, service: &str, args: Value) -> usize { + let heap = NEXT.fetch_add(1, Ordering::Relaxed); + let call = ServiceCall { app_id: APP.into(), service: service.into(), args, from_sheet: false, may_prompt: false, host_dir: self.dir.clone() }; + dispatch(call, heap, 1, &mut self.host); + heap + } + fn ask(&mut self, service: &str, args: Value) -> Result { let heap = self.send(APP, service, args, false); wait(heap) @@ -129,6 +138,24 @@ fn two_providers(rig: &mut Rig) { rig.add(json!({"family": "zai", "model": "", "base_url": "", "api_type": "default", "key": ZAI_KEY})); } +#[test] +fn a_background_surface_cannot_raise_or_replace_a_sheet() { + let mut rig = Rig::new("background", None); + two_providers(&mut rig); + let id = all(&rig.ask("llm.providers", json!({})).unwrap())[0]["id"].clone(); + // The person is adding a provider on the sheet in the foreground. + let waiting = rig.send(APP, "llm.add_provider", Value::Null, false); + let body = rig.host.body().to_string(); + for (method, args) in [("llm.add_provider", Value::Null), ("llm.edit_provider", json!({"id": id})), + ("llm.import_qr", Value::Null), ("llm.export_qr", Value::Null)] { + let heap = rig.send_background(method, args); + let refused = wait(heap).unwrap_err(); + assert!(refused.contains("Open AI providers"), "{method}: {refused}"); + } + assert_eq!(rig.host.body(), body, "the person's sheet is untouched"); + assert!(still_waiting(waiting), "and its request still waits for them"); +} + #[test] fn talk_to_octos_controls_require_their_own_trusted_sheet() { let mut rig = Rig::new("client-sheet", None); diff --git a/apps/mail/host-service/src/lib.rs b/apps/mail/host-service/src/lib.rs index 073ffe77..20d60795 100644 --- a/apps/mail/host-service/src/lib.rs +++ b/apps/mail/host-service/src/lib.rs @@ -481,6 +481,13 @@ impl HostService for MailService { reply.send(Ok(json!(mine))); } "add_account" => { + // A sign-in needs the person on Mail's sheet. From a surface + // that cannot show one (a home-screen tile, an agent's tool + // call), refuse before touching a sign-in already under way. + if !call.may_prompt { + reply.send(Err("Signing in needs Mail open: open Mail to add an account.".into())); + return; + } if let Some((_, earlier)) = self.pending.lock().unwrap().take() { earlier.send(Err("Another sign-in replaced this one.".into())); } @@ -911,6 +918,17 @@ mod tests { // add_account raises the sheet and waits; a wrong password keeps it waiting. let add = send(&dir, "os.mail", "mail.add_account", Value::Null, false, &mut host); assert!(matches!(host.sheet, Some(Some(_))), "the host's sheet is up"); + + // Mail's tile on the home screen cannot start a sign-in, and does + // not take over the one the person is filling in. + let tile = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let mut tile_host = Host::default(); + let background = ServiceCall { app_id: "os.mail".into(), service: "mail.add_account".into(), args: Value::Null, + from_sheet: false, may_prompt: false, host_dir: dir.clone() }; + octosense_appstore::services::dispatch(background, tile, 1, &mut tile_host); + assert!(wait(tile).unwrap_err().contains("open Mail"), "refused, with what to do"); + assert!(tile_host.sheet.is_none(), "no sheet over the tile"); + assert!(octosense_appstore::services::take_replies_for(&[add]).is_empty(), "the foreground sign-in is still waiting"); let form = json!({"address": "me@example.com", "password": "wrong", "host": "pop.example.com", "port": "995", "security": "tls", "smtp_host": "smtp.example.com", "smtp_port": "465", "smtp_security": "tls"}); assert!(ask(&dir, "os.mail", "mail.sheet.submit", form.clone(), true, &mut host).unwrap_err().contains("Wrong password")); diff --git a/crates/shell/src/glance_card.rs b/crates/shell/src/glance_card.rs index d39b3228..40a902ad 100644 --- a/crates/shell/src/glance_card.rs +++ b/crates/shell/src/glance_card.rs @@ -236,7 +236,14 @@ impl GlanceTiles { let gone: Vec = self.tiles.keys().filter(|k| !live.contains(k)).cloned().collect(); for key in gone { if let Some(tile) = self.tiles.remove(&key) { - tile.frame.splash(cx, ids!(card)).set_text(cx, ""); + let splash = tile.frame.splash(cx, ids!(card)); + // Its waiting host requests end with it: a late answer goes + // nowhere, never to a tile that takes its place. + #[cfg(any(feature = "app-hub", native_mobile))] + if let Some(heap) = splash.borrow_mut().and_then(|mut s| s.isolate_heap_key(cx)) { + octosense_appstore::services::cancel_heap(heap); + } + splash.set_text(cx, ""); } } } @@ -268,7 +275,8 @@ fn seat(cx: &mut Cx, splash: &SplashRef, app: &str, contained: bool) { /// The isolate settings of `app`'s policy, resolved as the Card runner /// resolves them when it opens the app: a system app's from its shipped -/// pack, any other from the last verified catalog. +/// pack, any other from the last verified catalog. A tile is a background +/// surface, so a service its card calls may not raise a sheet over it. #[cfg(any(feature = "app-hub", native_mobile))] pub fn app_isolate(cx: &Cx, app: &str) -> Result { let root = octosense_appstore::data_root(cx); @@ -282,7 +290,8 @@ pub fn app_isolate(cx: &Cx, app: &str) -> Result>>); + impl HostService for Holds { + fn family(&self) -> &'static str { + "glancetilehold" + } + fn call(&mut self, _call: ServiceCall, reply: Replier, _host: &mut dyn ServiceHost) { + *self.0.lock().unwrap() = Some(reply); + } + } + struct NoSheet; + impl ServiceHost for NoSheet { + fn open_sheet(&mut self, _: String) {} + fn close_sheet(&mut self) {} + } + register_test_app("os.glancesweep"); + let held = Arc::new(Mutex::new(None)); + register_host_service(Box::new(Holds(held.clone()))); + let mut cx = tile_cx(); + let mut tiles = GlanceTiles::default(); + let splash = tiles.open(&mut cx, "os.glancesweep/c", "os.glancesweep", true, &"View{}".into()); + let heap = heap_of(&mut cx, &splash); + let call = ServiceCall { app_id: "os.glancesweep".into(), service: "glancetilehold.wait".into(), args: serde_json::Value::Null, + from_sheet: false, may_prompt: false, host_dir: std::env::temp_dir() }; + dispatch(call, heap, 1, &mut NoSheet); + tiles.sweep(&mut cx, &[]); + held.lock().unwrap().take().expect("the service holds the request").send(Ok(serde_json::Value::Null)); + assert!(take_replies_for(&[heap]).is_empty(), "nothing is left queued for the tile that went away"); + } + #[test] fn heights_clamp_to_the_tile_range() { assert_eq!(clamp_height(10.0), TILE_MIN_HEIGHT); diff --git a/crates/shell/src/host_tools/script_apps.rs b/crates/shell/src/host_tools/script_apps.rs index 08534745..e5ffbc1e 100644 --- a/crates/shell/src/host_tools/script_apps.rs +++ b/crates/shell/src/host_tools/script_apps.rs @@ -161,6 +161,7 @@ pub struct HostServiceExecutor { /// Where a call's answer goes: App Hub's reply queue, keyed by a heap key no /// isolate uses. struct Waiting { + call_id: String, reply: ToolReply, } @@ -190,13 +191,29 @@ impl ToolExecutor for HostServiceExecutor { return; } let key = NEXT_KEY.fetch_add(1, std::sync::atomic::Ordering::Relaxed); - WAITING.lock().unwrap_or_else(|e| e.into_inner()).get_or_insert_with(HashMap::new).insert(key, Waiting { reply }); - let service_call = ServiceCall { app_id: self.app.clone(), service: call.name.clone(), args: call.args.clone(), from_sheet: false, may_prompt: true, host_dir: self.host_dir.clone() }; + WAITING.lock().unwrap_or_else(|e| e.into_inner()).get_or_insert_with(HashMap::new).insert(key, Waiting { call_id: call.call_id.clone(), reply }); + let service_call = ServiceCall { app_id: self.app.clone(), service: call.name.clone(), args: call.args.clone(), from_sheet: false, + // A tool call has no surface for a sheet: the person is not in the app. + may_prompt: false, host_dir: self.host_dir.clone() }; octosense_appstore::services::dispatch(service_call, key, 0, &mut NoSheet); } - fn cancel(&self, _call_id: &str) { - // The reply is closed: whatever the service answers is dropped. + fn cancel(&self, call_id: &str) { + // Stop waiting now, and have App Hub drop the request: the service's + // late answer goes nowhere, and the request no longer counts against + // the calls that may wait. + let keys: Vec = { + let mut waiting = WAITING.lock().unwrap_or_else(|e| e.into_inner()); + let Some(waiting) = waiting.as_mut() else { return }; + let keys: Vec = waiting.iter().filter(|(_, w)| w.call_id == call_id).map(|(key, _)| *key).collect(); + for key in &keys { + waiting.remove(key); + } + keys + }; + for key in keys { + octosense_appstore::services::cancel_heap(key); + } } } @@ -213,8 +230,8 @@ pub fn poll() { Err(message) => ToolOutcome::error("app_error", message), }); } - // A call cancelled while its service worked stays until the service - // answers (its reply, closed, sends nothing). + // A call its service never answers times out in App Hub, and the + // timeout arrives here like any answer. } /// How many calls wait on a host service. @@ -351,6 +368,42 @@ pub(crate) mod tests { (ToolReply::new("c", move |v| s.lock().unwrap().push(v)), sent) } + struct Holds(Arc>>); + impl HostService for Holds { + fn family(&self) -> &'static str { + "g3hold" + } + fn call(&mut self, _call: ServiceCall, reply: Replier, _host: &mut dyn ServiceHost) { + *self.0.lock().unwrap() = Some(reply); + } + } + + fn is_waiting(call_id: &str) -> bool { + WAITING.lock().unwrap().as_ref().is_some_and(|w| w.values().any(|waiting| waiting.call_id == call_id)) + } + + /// A cancelled tool call stops waiting at once, and its service's late + /// answer goes nowhere. + #[test] + fn a_cancelled_tool_call_stops_waiting() { + let held = Arc::new(Mutex::new(None)); + octosense_appstore::services::register_host_service(Box::new(Holds(held.clone()))); + let exec = HostServiceExecutor { + app: "os.g3hold".into(), + tools: ["g3hold.wait".to_string()].into_iter().collect(), + families: ["g3hold".to_string()].into_iter().collect(), + host_dir: std::env::temp_dir(), + }; + let (r, sent) = reply(); + exec.execute(call("g3hold.wait"), r); + assert!(is_waiting("c-g3hold.wait")); + exec.cancel("c-g3hold.wait"); + assert!(!is_waiting("c-g3hold.wait"), "cancelled"); + held.lock().unwrap().take().expect("the service held it").send(Ok(json!({}))); + poll(); + assert!(sent.lock().unwrap().is_empty(), "the late answer reached nobody"); + } + /// The executor runs a host-service tool as the app's own /// `host.request` would (its identity, never a sheet), only for a /// granted family; the answer reaches the call through `poll`.