From 4d6246e6137afb2f58a4c89cae4b59ef4baa98aa Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 11:15:47 -0700 Subject: [PATCH 1/4] fix(marketplace): match a search query word by word, and explain an empty result (F5) The 2026-09-10 composer QA run measured `skills__searchMarketplaceSkills {query: "R scripting ggplot visualization"}` -> `total: 0` against a live registry where `ggplot` alone found 2 and `r-scripting` found 1. The matcher asked whether the WHOLE lowercased query was a substring of one field, so any phrase failed, and the model told the user the marketplace had nothing. Both marketplace searches (skills and extensions) shared that matcher, so both move onto `marketplace/search.rs`: the query is split into terms at whitespace and punctuation, an entry matching ANY term is a hit, and hits are ranked by a verbatim match (what the old matcher found, so nothing is lost), then by how many terms matched, then by where (id/name > labels > description). Terms under three characters match whole words only, so `r` finds the R language rather than every word containing an r, and filler words are dropped. Each hit now carries `matchedTerms`, the result carries the `terms` the query was read as, and a query that still matches nothing returns `guidance` saying how many entries the registry holds and to try shorter terms. The extension count is what the caller may see, never the registry's size, so a public model is never told how many private rows exist. --- .../src/agents/extension_manager_extension.rs | 177 ++++++- .../biorouter/src/agents/skills_extension.rs | 176 ++++++- crates/biorouter/src/marketplace.rs | 270 +++++++++-- crates/biorouter/src/marketplace/search.rs | 433 ++++++++++++++++++ 4 files changed, 971 insertions(+), 85 deletions(-) create mode 100644 crates/biorouter/src/marketplace/search.rs diff --git a/crates/biorouter/src/agents/extension_manager_extension.rs b/crates/biorouter/src/agents/extension_manager_extension.rs index e3f7ff13f..493dda31d 100644 --- a/crates/biorouter/src/agents/extension_manager_extension.rs +++ b/crates/biorouter/src/agents/extension_manager_extension.rs @@ -421,6 +421,87 @@ fn marketplace_descriptor_json( payload } +/// The `search_marketplace_extensions` result, from a catalog already loaded — +/// split from the load so it is testable without the network. +/// +/// `caller` is the admitted capability's tier. Everything below — the hits, the +/// browse list and the count the guidance quotes — is taken from what that +/// caller may see, so a public model's answer never counts a private row. +fn marketplace_extensions_json( + loaded: &crate::marketplace::MarketplaceCatalogLoad, + query: Option<&str>, + caller: crate::privacy::ProviderTier, +) -> Value { + let visible = loaded.catalog.browse_extensions(caller); + // Ranked, and each hit says which query terms it matched: the search is + // shared with the skills catalog (finding F5). + let search = query.map(|query| loaded.catalog.search_extensions(caller, query)); + let extensions: Vec = match &search { + Some(search) => search + .hits + .iter() + .map(|hit| { + let mut payload = marketplace_descriptor_json(hit.entry); + if let Some(fields) = payload.as_object_mut() { + fields.insert( + "matchedTerms".to_owned(), + serde_json::json!(hit.matched_terms), + ); + } + payload + }) + .collect(), + None => visible + .iter() + .copied() + .map(marketplace_descriptor_json) + .collect(), + }; + let source = match loaded.source { + crate::marketplace::MarketplaceCatalogSource::Live => "live", + crate::marketplace::MarketplaceCatalogSource::LastGood => "lastGood", + crate::marketplace::MarketplaceCatalogSource::Embedded => "embedded", + }; + let mut body = serde_json::json!({ + "source": source, + "stale": loaded.is_stale(), + "extensions": extensions, + }); + if let (Some(query), Some(search), Some(fields)) = (query, &search, body.as_object_mut()) { + fields.insert("terms".to_owned(), serde_json::json!(&search.terms)); + if search.is_empty() { + fields.insert( + "guidance".to_owned(), + Value::String(no_marketplace_extension_matched( + &search.describe_query(query), + visible.len(), + )), + ); + } + } + body +} + +/// What an empty `search_marketplace_extensions` says instead of an empty list +/// — the extension half of finding F5, whose skills half let a model tell a +/// user the marketplace had nothing. +/// +/// ⚠ `visible` is the count shown to THIS caller. For a public model that is +/// the public rows only; the registry's full size would count the private +/// extensions Gate E keeps out of its sight. +fn no_marketplace_extension_matched(asked: &str, visible: usize) -> String { + let available = match visible { + 1 => "1 extension is".to_owned(), + n => format!("{n} extensions are"), + }; + format!( + "No marketplace extension available to this model matched {asked}. {available} \ + available to this model, so this does not mean there is nothing relevant: try a \ + shorter or more general term (one tool, data source or topic name), or call \ + search_marketplace_extensions with no query to list them all." + ) +} + fn marketplace_approval_request( mutation: MarketplaceMutation, descriptor: &crate::marketplace::MarketplaceExtensionDescriptor, @@ -2241,23 +2322,7 @@ impl ExtensionManagerClient { .map_err(|error| ExtensionManagerToolError::OperationFailed { message: error.to_string(), })?; - let entries = match query { - Some(query) => loaded.catalog.search_extensions(cap.tier(), query), - None => loaded.catalog.browse_extensions(cap.tier()), - }; - let source = match loaded.source { - crate::marketplace::MarketplaceCatalogSource::Live => "live", - crate::marketplace::MarketplaceCatalogSource::LastGood => "lastGood", - crate::marketplace::MarketplaceCatalogSource::Embedded => "embedded", - }; - let body = serde_json::json!({ - "source": source, - "stale": loaded.is_stale(), - "extensions": entries - .into_iter() - .map(marketplace_descriptor_json) - .collect::>(), - }); + let body = marketplace_extensions_json(&loaded, query, cap.tier()); Ok(vec![Content::text( serde_json::to_string_pretty(&body).unwrap_or_else(|_| "{}".to_owned()), )]) @@ -3267,7 +3332,7 @@ impl ExtensionManagerClient { tools.extend([ Tool::new( SEARCH_MARKETPLACE_EXTENSIONS_TOOL_NAME.to_owned(), - "Browse or search trusted BAAM marketplace extensions. Pass `query` to match an id, name, organization, description or tag; omit it to list everything visible to this model. Private entries are hidden from public models. Results carry `registryId` (camelCase); pass that exact value as install_extension's `registry_id` (snake_case) — the two tools spell the same field differently." + "Browse or search trusted BAAM marketplace extensions. Pass `query` to search ids, names, organizations, descriptions and tags — an entry matching any of its words is returned, best match first; omit it to list everything visible to this model. Private entries are hidden from public models. Results carry `registryId` (camelCase); pass that exact value as install_extension's `registry_id` (snake_case) — the two tools spell the same field differently." .to_owned(), Arc::new( serde_json::to_value(schema_for!(SearchMarketplaceExtensionsParams)) @@ -3572,6 +3637,82 @@ mod tests { .unwrap() } + /// The extension half of finding F5, at the tool's output: a phrase is + /// matched by any of its words and ranked, and a query matching nothing + /// explains itself instead of returning an empty list. + /// + /// ⚠ The explanation counts what THIS caller may see. A public caller + /// whose query matches only the private row it is not shown must read + /// exactly what a query matching nothing at all reads — otherwise the + /// guidance, not the hit list, becomes the private catalog's oracle. + #[test] + fn marketplace_search_ranks_a_phrase_and_explains_an_empty_result_from_visible_rows() { + let loaded = crate::marketplace::MarketplaceCatalogLoad { + catalog: marketplace_catalog(), + source: crate::marketplace::MarketplaceCatalogSource::Embedded, + cache_warning: None, + }; + let ids = |body: &Value| -> Vec { + body["extensions"] + .as_array() + .unwrap() + .iter() + .map(|entry| entry["registryId"].as_str().unwrap().to_owned()) + .collect() + }; + + let as_public = marketplace_extensions_json( + &loaded, + Some("public fixture manager"), + ProviderTier::Public, + ); + assert_eq!( + as_public["terms"], + serde_json::json!(["public", "fixture", "manager"]) + ); + assert_eq!(ids(&as_public), ["manager-public-fixture"]); + assert_eq!( + as_public["extensions"][0]["matchedTerms"], + serde_json::json!(["public", "fixture", "manager"]) + ); + let as_private = marketplace_extensions_json( + &loaded, + Some("public fixture manager"), + ProviderTier::Private, + ); + assert_eq!( + ids(&as_private), + ["manager-public-fixture", "manager-private-fixture"], + "the row matching every term ranks first" + ); + + let guidance = |query: &str| -> String { + let body = marketplace_extensions_json(&loaded, Some(query), ProviderTier::Public); + assert!(ids(&body).is_empty(), "{body}"); + body["guidance"] + .as_str() + .expect("an empty result explains itself") + .replace(&format!("`{query}`"), "`QUERY`") + }; + let hidden_match = guidance("private"); + assert!( + hidden_match.contains("1 extension is available to this model"), + "{hidden_match}" + ); + assert_eq!( + hidden_match, + guidance("zzqx"), + "a public caller's miss on a hidden private row reads differently from a miss on \ + nothing" + ); + assert!( + marketplace_extensions_json(&loaded, None, ProviderTier::Public) + .get("guidance") + .is_none(), + "browsing needs no explanation" + ); + } + #[test] fn install_schema_accepts_only_a_registry_id_and_enable_flag() { let schema = serde_json::to_value(schema_for!(InstallExtensionParams)).unwrap(); diff --git a/crates/biorouter/src/agents/skills_extension.rs b/crates/biorouter/src/agents/skills_extension.rs index 3260c1c4c..2ecd74f41 100644 --- a/crates/biorouter/src/agents/skills_extension.rs +++ b/crates/biorouter/src/agents/skills_extension.rs @@ -38,7 +38,7 @@ const SKILL_OPERATION_GUIDANCE: &[(&str, &str)] = &[ ("loadSkill", "loadSkill reads an exact installed skill"), ( "searchMarketplaceSkills", - "searchMarketplaceSkills lists trusted BAAM entries, or filters them when you pass a query", + "searchMarketplaceSkills lists trusted BAAM entries, or ranks those matching any word of a query you pass", ), ( "installMarketplaceSkill", @@ -1786,20 +1786,51 @@ impl SkillsClient { let loaded = crate::marketplace::load_marketplace_catalog() .await .map_err(|error| error.to_string())?; + Ok(vec![Content::text( + Self::marketplace_skill_page_json(&loaded, query, offset, limit).to_string(), + )]) + } + + /// One page of `searchMarketplaceSkills`, from a catalog already loaded — + /// split from the load so the page is testable without the network. + fn marketplace_skill_page_json( + loaded: &crate::marketplace::MarketplaceCatalogLoad, + query: Option<&str>, + offset: usize, + limit: usize, + ) -> serde_json::Value { let source = Self::marketplace_source_name(loaded.source); let stale = loaded.is_stale(); let cache_warning = loaded.cache_warning.clone(); - let matches = match query { - Some(query) => loaded.catalog.search_skills(query), - None => loaded.catalog.browse_skills(), + let registry_size = loaded.catalog.browse_skills().len(); + // A query is ranked, not filtered (finding F5): its terms are matched + // separately and each hit says which of them it matched, so a model + // reading a long list can tell an entry that matched every term from + // one that matched a single common word. + let search = query.map(|query| loaded.catalog.search_skills(query)); + let matches: Vec<( + &crate::marketplace::MarketplaceSkillDescriptor, + Option<&[String]>, + )> = match &search { + Some(search) => search + .hits + .iter() + .map(|hit| (hit.entry, Some(hit.matched_terms.as_slice()))) + .collect(), + None => loaded + .catalog + .browse_skills() + .into_iter() + .map(|entry| (entry, None)) + .collect(), }; let total = matches.len(); let entries: Vec<_> = matches .into_iter() .skip(offset) .take(limit) - .map(|entry| { - serde_json::json!({ + .map(|(entry, matched_terms)| { + let mut row = serde_json::json!({ "registryId": &entry.registry_id, "name": &entry.name, "category": &entry.category, @@ -1808,25 +1839,58 @@ impl SkillsClient { "tags": &entry.tags, "keywords": &entry.keywords, "license": &entry.license, - }) + }); + if let (Some(matched_terms), Some(fields)) = (matched_terms, row.as_object_mut()) { + fields.insert("matchedTerms".to_owned(), serde_json::json!(matched_terms)); + } + row }) .collect(); let returned = entries.len(); let next_offset = (offset + returned < total).then_some(offset + returned); - Ok(vec![Content::text( - serde_json::json!({ - "source": source, - "stale": stale, - "cacheWarning": cache_warning, - "total": total, - "offset": offset, - "limit": limit, - "returned": returned, - "nextOffset": next_offset, - "skills": entries, - }) - .to_string(), - )]) + let mut body = serde_json::json!({ + "source": source, + "stale": stale, + "cacheWarning": cache_warning, + "total": total, + "offset": offset, + "limit": limit, + "returned": returned, + "nextOffset": next_offset, + "skills": entries, + }); + if let (Some(query), Some(search), Some(fields)) = (query, &search, body.as_object_mut()) { + fields.insert("terms".to_owned(), serde_json::json!(&search.terms)); + if search.is_empty() { + fields.insert( + "guidance".to_owned(), + serde_json::Value::String(Self::no_marketplace_skill_matched( + &search.describe_query(query), + registry_size, + )), + ); + } + } + body + } + + /// What an empty marketplace search says instead of a bare `total: 0` + /// (finding F5). That answer let a model report *"no matching marketplace + /// skills found"* about a registry that held every skill the user named, + /// so the guidance says how big the registry is and what to try next — a + /// miss is more often the query's wording than the registry's contents. + fn no_marketplace_skill_matched(asked: &str, registry_size: usize) -> String { + let skills = if registry_size == 1 { + "skill" + } else { + "skills" + }; + format!( + "No marketplace skill matched {asked}. The registry holds {registry_size} {skills}, \ + so this does not mean there is nothing relevant: try a shorter or more general term \ + (one tool, language or topic name), or call searchMarketplaceSkills with no query \ + to list them all." + ) } /// Browse or search the trusted BAAM skill registry. @@ -2793,8 +2857,10 @@ impl SkillsClient { indoc! {r#" Browse or search the trusted skill entries published in BAAM. - Pass `query` to match an id, name, category, description, tag or keyword; - omit it to list the whole registry. This returns registry ids and metadata, + Pass `query` to search ids, names, categories, descriptions, tags and keywords: + an entry matching any of its words is returned, the entries matching the most + words first, each with the `matchedTerms` it matched. Omit `query` to list the + whole registry. This returns registry ids and metadata, never arbitrary download URLs — pass an exact returned registryId as installMarketplaceSkill's registry_id. "#} @@ -5938,6 +6004,70 @@ mod merged_surface_tests { } } + /// Finding F5 at the tool's own output. The QA run read `total: 0` for the + /// phrase below against a registry holding every skill it names, and the + /// model reported "no matching marketplace skills found". The phrase now + /// finds them, each hit says which terms it matched, and a query that + /// genuinely matches nothing explains itself instead of returning a bare + /// zero. + #[test] + fn a_marketplace_skill_page_ranks_a_phrase_and_explains_an_empty_result() { + let loaded = crate::marketplace::MarketplaceCatalogLoad::embedded_for_test(); + let registry = loaded.catalog.browse_skills().len(); + + let page = SkillsClient::marketplace_skill_page_json( + &loaded, + Some("R scripting ggplot visualization"), + 0, + 50, + ); + assert!(page["total"].as_u64().unwrap() >= 2, "{page}"); + assert_eq!( + page["terms"], + serde_json::json!(["r", "scripting", "ggplot", "visualization"]) + ); + let skills = page["skills"].as_array().unwrap(); + let ids: Vec<&str> = skills + .iter() + .map(|skill| skill["registryId"].as_str().unwrap()) + .collect(); + assert!( + ids.contains(&"ggplot-visualization") && ids.contains(&"r-scripting"), + "{ids:?}" + ); + assert!( + skills + .iter() + .all(|skill| !skill["matchedTerms"].as_array().unwrap().is_empty()), + "{page}" + ); + assert!(page.get("guidance").is_none(), "{page}"); + + let none = SkillsClient::marketplace_skill_page_json(&loaded, Some("zzqx"), 0, 50); + assert_eq!(none["total"], 0); + let guidance = none["guidance"] + .as_str() + .expect("an empty result explains itself"); + assert!( + guidance.contains(&format!("The registry holds {registry} skills")), + "{guidance}" + ); + assert!( + guidance.contains("`zzqx`") && guidance.contains("shorter"), + "{guidance}" + ); + + // Browsing is untouched: no terms, no matchedTerms, no guidance. + let browse = SkillsClient::marketplace_skill_page_json(&loaded, None, 0, 5); + assert_eq!(browse["total"], registry); + assert!(browse.get("terms").is_none(), "{browse}"); + assert!( + browse["skills"][0].get("matchedTerms").is_none(), + "{browse}" + ); + assert!(browse.get("guidance").is_none(), "{browse}"); + } + /// The retired names keep dispatching. They are not advertised — that is /// the whole point — but a persisted transcript, a stored `always allow` /// grant, or a coding-agent child that read one still calls them, and an diff --git a/crates/biorouter/src/marketplace.rs b/crates/biorouter/src/marketplace.rs index 54e1f78c7..e038fe503 100644 --- a/crates/biorouter/src/marketplace.rs +++ b/crates/biorouter/src/marketplace.rs @@ -12,6 +12,11 @@ use crate::config::paths::Paths; use crate::privacy::affiliation::InstitutionId; use crate::privacy::{ExtensionAffiliation, ProviderTier}; +mod search; + +use search::Weight; +pub use search::{MarketplaceSearch, MarketplaceSearchHit}; + pub const REGISTRY_URL: &str = "https://biorouter.ucsf.edu/registry.json"; const REGISTRY_SOURCE: &str = "https://biorouter.ucsf.edu/baam"; pub const MAX_REGISTRY_BYTES: usize = 2 * 1024 * 1024; @@ -109,56 +114,58 @@ impl MarketplaceCatalog { .collect() } + /// Rank the extensions visible to `caller` against a free-text query. How a + /// query is matched — and why a phrase is a union of its words rather than + /// one substring (finding F5) — is documented in `marketplace/search.rs`. + /// + /// The caller filter runs FIRST, so an extension hidden from `caller` is + /// never scored and cannot move, or be counted among, what it is shown. pub fn search_extensions( &self, caller: ProviderTier, query: &str, - ) -> Vec<&MarketplaceExtensionDescriptor> { - let query = query.trim().to_ascii_lowercase(); - self.browse_extensions(caller) - .into_iter() - .filter(|entry| { - query.is_empty() - || searchable( - &query, - [ - entry.registry_id.as_str(), - entry.extension_name.as_str(), - entry.name.as_str(), - entry.organization.as_str(), - entry.description.as_str(), - ] - .into_iter() - .chain(entry.tags.iter().map(String::as_str)), - ) - }) - .collect() + ) -> MarketplaceSearch<'_, MarketplaceExtensionDescriptor> { + search::rank( + query, + search::EXTENSION_NOISE, + self.browse_extensions(caller), + |entry| { + let mut fields = vec![ + (entry.registry_id.as_str(), Weight::Name), + (entry.extension_name.as_str(), Weight::Name), + (entry.name.as_str(), Weight::Name), + (entry.organization.as_str(), Weight::Label), + (entry.description.as_str(), Weight::Prose), + ]; + fields.extend(entry.tags.iter().map(|tag| (tag.as_str(), Weight::Label))); + fields + }, + ) } pub fn browse_skills(&self) -> Vec<&MarketplaceSkillDescriptor> { self.skills.values().collect() } - pub fn search_skills(&self, query: &str) -> Vec<&MarketplaceSkillDescriptor> { - let query = query.trim().to_ascii_lowercase(); - self.skills - .values() - .filter(|entry| { - query.is_empty() - || searchable( - &query, - [ - entry.registry_id.as_str(), - entry.name.as_str(), - entry.category.as_str(), - entry.description.as_str(), - ] - .into_iter() - .chain(entry.tags.iter().map(String::as_str)) - .chain(entry.keywords.iter().map(String::as_str)), - ) - }) - .collect() + /// Rank every skill against a free-text query, matched as documented in + /// `marketplace/search.rs`. + pub fn search_skills(&self, query: &str) -> MarketplaceSearch<'_, MarketplaceSkillDescriptor> { + search::rank(query, search::SKILL_NOISE, self.skills.values(), |entry| { + let mut fields = vec![ + (entry.registry_id.as_str(), Weight::Name), + (entry.name.as_str(), Weight::Name), + (entry.category.as_str(), Weight::Label), + (entry.description.as_str(), Weight::Prose), + ]; + fields.extend(entry.tags.iter().map(|tag| (tag.as_str(), Weight::Label))); + fields.extend( + entry + .keywords + .iter() + .map(|keyword| (keyword.as_str(), Weight::Label)), + ); + fields + }) } pub fn resolve_extension_for_install( @@ -219,10 +226,6 @@ impl MarketplaceCatalog { } } -fn searchable<'a>(query: &str, mut fields: impl Iterator) -> bool { - fields.any(|field| field.to_ascii_lowercase().contains(query)) -} - #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] struct RawRegistry { @@ -565,6 +568,19 @@ impl MarketplaceCatalogLoad { pub fn is_stale(&self) -> bool { self.source != MarketplaceCatalogSource::Live } + + /// The registry snapshot shipped in the binary, loaded as the tools see it + /// offline — for tests of what a tool builds from a catalog, which must not + /// depend on the network. + #[cfg(test)] + pub(crate) fn embedded_for_test() -> Self { + Self { + catalog: MarketplaceCatalog::from_bytes(EMBEDDED_REGISTRY) + .expect("the shipped registry is a valid catalog"), + source: MarketplaceCatalogSource::Embedded, + cache_warning: None, + } + } } pub async fn load_marketplace_catalog() -> Result { @@ -877,6 +893,172 @@ mod tests { ); } + /// Seven skill rows copied VERBATIM from `landing/registry.json` at + /// 7c96d796, the registry the 2026-09-10 composer QA run measured finding + /// F5 against. Frozen here rather than read from [`EMBEDDED_REGISTRY`] so + /// the exact rankings below cannot drift when the registry gains a skill; + /// the shipped-registry test after them pins only the measured shape. + fn skills_snapshot() -> Vec { + fn row( + id: &str, + name: &str, + category: &str, + kind: &str, + description: &str, + tags: &[&str], + keywords: &[&str], + ) -> serde_json::Value { + json!({ + "id": id, + "name": name, + "category": category, + "type": kind, + "description": description, + "tags": tags, + "keywords": keywords, + "download": format!("https://github.com/BaranziniLab/biorouter-skills/releases/download/skill-{id}/{id}.zip"), + "filename": format!("{id}.zip"), + "license": "Apache-2.0" + }) + } + serde_json::to_vec(&json!({ + "version": 2, + "source": "https://biorouter.ucsf.edu/baam", + "institutions": { "ucsf": "UCSF" }, + "extensions": [], + "skills": [ + row("data-visualization", "Data Visualization", "Biomedical", "13 skills · auto-applied", + "Publication-quality plots: heatmaps, volcano, Manhattan, dimplots.", + &["ggplot2", "matplotlib", "ComplexHeatmap"], + &["data-visualization", "ggplot2", "matplotlib", "complexheatmap"]), + row("ggplot-visualization", "ggplot2 Visualization", "Core", "Auto-applied · R plotting", + "Applies ggplot2 best-practice style when writing R plotting code.", + &["R", "ggplot2"], &[]), + row("python-scripting", "Python Scripting", "Core", "Auto-applied · Python code", + "Applies Python naming, typing, error handling, and project structure conventions when writing Python code.", + &["Python"], &[]), + row("r-scripting", "R Scripting", "Core", "Auto-applied · R code", + "Applies tidyverse conventions and documentation standards when writing or reviewing R code.", + &["R", "Tidyverse"], &[]), + row("scientific-visual-communication", "Scientific Visual Communication", "Core", + "User-invocable · /scientific-visual-communication", + "Plans schematics, posters, slides, figure panels, infographics, visual abstracts, and source-to-visual traceability.", + &["Visuals", "Posters", "Apache-2.0"], + &["scientific-visual-communication", "schematics", "infographics", "posters", "slides", "visual", "abstracts", "apache"]), + row("clinical-biostatistics", "Clinical Biostatistics", "Biomedical", "6 skills · auto-applied", + "Survival, mixed models, and clinical-trial statistical analysis.", + &["survival", "R", "lme4"], &["clinical-biostatistics", "survival", "r", "lme4"]), + row("single-cell", "Single-cell", "Biomedical", "14 skills · auto-applied", + "scRNA-seq clustering, annotation, trajectory, and integration.", + &["Scanpy", "Seurat", "scVI"], &["single-cell", "scanpy", "seurat", "scvi"]), + ] + })) + .unwrap() + } + + fn skill_ids(search: &MarketplaceSearch<'_, MarketplaceSkillDescriptor>) -> Vec { + search + .hits + .iter() + .map(|hit| hit.entry.registry_id.clone()) + .collect() + } + + /// Finding F5, the three queries the QA run measured. The unfixed matcher + /// asked whether the whole query was a substring of one field, so the + /// phrase returned `total: 0` while each of its words, asked alone, found + /// the skills it names. It now returns their union, ranked: the skill + /// matching three of the four terms, then the two matching two, then the + /// single-term matches. + #[test] + fn a_natural_language_skill_query_returns_the_union_ranked() { + let catalog = MarketplaceCatalog::from_bytes(&skills_snapshot()).unwrap(); + + let phrase = catalog.search_skills("R scripting ggplot visualization"); + assert_eq!(phrase.terms, ["r", "scripting", "ggplot", "visualization"]); + assert_eq!( + skill_ids(&phrase), + [ + "ggplot-visualization", + "r-scripting", + "data-visualization", + "python-scripting", + "clinical-biostatistics", + ], + "measured before this fix: no hits at all" + ); + assert_eq!( + phrase.hits[0].matched_terms, + ["r", "ggplot", "visualization"] + ); + assert!( + !skill_ids(&phrase).contains(&"scientific-visual-communication".to_owned()), + "`visual` is not `visualization`: a long term must be found in the entry, not the \ + other way round" + ); + + // The two single-term controls, measured in the same chat as 2 and 1. + assert_eq!( + skill_ids(&catalog.search_skills("ggplot")), + ["ggplot-visualization", "data-visualization"] + ); + let id = catalog.search_skills("r-scripting"); + assert_eq!( + skill_ids(&id)[0], + "r-scripting", + "the skill the query names ranks first, ahead of the other skills about R or \ + scripting" + ); + assert_eq!(id.hits[0].matched_terms, ["r", "scripting"]); + + // The empty query stays the browse case, 129 of 129 in the QA run. + assert_eq!( + catalog.search_skills("").len(), + catalog.browse_skills().len() + ); + } + + /// The same property against the registry that ships in the binary — the + /// offline fallback, and the snapshot of the live registry the QA run read. + /// Only the measured SHAPE is pinned, so a new skill cannot fail it: the + /// phrase finds every skill that each of its words finds alone. + #[test] + fn on_the_shipped_registry_a_phrase_finds_what_each_of_its_words_finds() { + let catalog = MarketplaceCatalog::from_bytes(EMBEDDED_REGISTRY).unwrap(); + let phrase = skill_ids(&catalog.search_skills("R scripting ggplot visualization")); + for word in ["ggplot", "r-scripting", "scripting", "visualization"] { + for id in skill_ids(&catalog.search_skills(word)) { + assert!( + phrase.contains(&id), + "`{word}` alone finds `{id}`, the phrase containing it does not: {phrase:?}" + ); + } + } + assert!(phrase.len() >= 2, "{phrase:?}"); + } + + /// The extension catalog shares the matcher, and the caller filter runs + /// before the ranking: a public caller's multi-word query can match the + /// private row's words and still never be shown it. + #[test] + fn a_multi_word_extension_query_ranks_only_what_the_caller_may_see() { + let catalog = MarketplaceCatalog::from_bytes(®istry("public")).unwrap(); + let ids = |caller| -> Vec { + catalog + .search_extensions(caller, "private fixture") + .hits + .iter() + .map(|hit| hit.entry.registry_id.clone()) + .collect() + }; + assert_eq!(ids(ProviderTier::Public), ["public-agent"]); + assert_eq!( + ids(ProviderTier::Private), + ["private-agent", "public-agent"], + "the row matching both terms ranks first" + ); + } + #[test] fn a_later_public_registry_row_cannot_lower_learned_private_authority() { let mut lowered: serde_json::Value = serde_json::from_slice(®istry("public")).unwrap(); diff --git a/crates/biorouter/src/marketplace/search.rs b/crates/biorouter/src/marketplace/search.rs new file mode 100644 index 000000000..52dacae22 --- /dev/null +++ b/crates/biorouter/src/marketplace/search.rs @@ -0,0 +1,433 @@ +//! Free-text search over the trusted marketplace catalog — the matcher behind +//! both `skills__searchMarketplaceSkills` and +//! `extensionmanager__search_marketplace_extensions`. +//! +//! ⚠ **A query is a set of words, not a substring.** The matcher this replaced +//! asked whether the WHOLE lowercased query occurred inside a single field, so a +//! one-word query worked and every phrase failed. Measured in the 2026-09-10 +//! composer QA run (finding F5), one chat, one live registry: +//! `R scripting ggplot visualization` → `total: 0`, while `ggplot` → 2 and +//! `r-scripting` → 1. A model composes exactly that phrase on a user's behalf, +//! so the shape that failed was the common one, and the model went on to tell +//! the user the marketplace had nothing. +//! +//! So a query is split into terms and an entry is a hit when it matches ANY of +//! them. The union is deliberate: no single entry has to contain every word a +//! user happened to say, and an AND over a phrase is the same empty answer with +//! a different cause. Precision comes from the ranking instead, best first: +//! +//! 1. an entry containing the query **verbatim** — what the old matcher found, +//! so nothing it returned is lost; +//! 2. then by **how many terms** it matched, so an entry matching every term +//! precedes one matching some; +//! 3. then by **where** each term matched — the id or name outweighs a tag, +//! which outweighs the description — and how exactly (the whole word, the +//! start of one, or inside one); +//! 4. then registry order, which is by id, so a result never reshuffles. +//! +//! Two rules keep the union from drowning the useful hits, and both were needed +//! by the measured query itself: +//! +//! * **A term under three characters matches whole words only.** `r` has to +//! find the R language; as a substring it matched nearly every entry. +//! * **Filler words are dropped** ("a skill about R" is `r`), because in a union +//! a word like `for` or `about` inflates the term count of every entry whose +//! prose happens to use it, which ranked noise above the real hit. + +/// How much a match in one field says about an entry. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) enum Weight { + /// Free prose: a description. + Prose = 1, + /// Curated labels: tags, keywords, a category, an organization. + Label = 2, + /// What the entry is called: its registry id and names. + Name = 3, +} + +/// Words that say how a request is phrased, not what it is for. Dropped from a +/// query unless nothing else is left, so a query made only of them (`agent`, +/// `or`) still searches for what it says. +const FILLER: &[&str] = &[ + "a", + "about", + "an", + "and", + "any", + "are", + "baam", + "be", + "by", + "can", + "do", + "does", + "find", + "for", + "from", + "help", + "how", + "i", + "in", + "into", + "is", + "it", + "its", + "looking", + "marketplace", + "me", + "my", + "need", + "of", + "on", + "or", + "please", + "search", + "some", + "that", + "the", + "this", + "to", + "use", + "using", + "via", + "want", + "what", + "which", + "with", +]; + +/// Filler specific to the skills catalog: every entry in it is a skill. +pub(super) const SKILL_NOISE: &[&str] = &["skill", "skills"]; + +/// Filler specific to the extensions catalog: every entry in it is an extension. +pub(super) const EXTENSION_NOISE: &[&str] = &["extension", "extensions"]; + +/// Below this many characters a term matches whole words only. +const MIN_PARTIAL_CHARS: usize = 3; + +/// One entry a search returned, with the query terms it matched. +#[derive(Debug)] +pub struct MarketplaceSearchHit<'a, T> { + pub entry: &'a T, + /// The terms this entry matched, in query order. Empty only for an entry + /// that nothing but the verbatim query found. + pub matched_terms: Vec, +} + +/// A ranked search: the terms the query was split into, and every entry that +/// matched at least one of them (or the whole query verbatim), best first. +#[derive(Debug)] +pub struct MarketplaceSearch<'a, T> { + /// What the query was read as, after filler words were dropped. Reported + /// to the model so a surprising result can be traced to its input. + pub terms: Vec, + pub hits: Vec>, +} + +impl MarketplaceSearch<'_, T> { + pub fn len(&self) -> usize { + self.hits.len() + } + + pub fn is_empty(&self) -> bool { + self.hits.is_empty() + } + + /// What was searched for, as a sentence fragment for an empty result: + /// "any of the terms `r`, `ggplot`", or "the query `…`" when the query + /// held no word at all. + pub fn describe_query(&self, query: &str) -> String { + if self.terms.is_empty() { + return format!("the query `{query}`"); + } + let terms = self + .terms + .iter() + .map(|term| format!("`{term}`")) + .collect::>() + .join(", "); + format!("any of the terms {terms}") + } +} + +/// Lowercase words, split at every character that is not a letter or digit — +/// whitespace and punctuation alike, so `r-scripting` is `r` + `scripting` and +/// `ggplot2` stays one word. +fn words(text: &str) -> impl Iterator + '_ { + text.split(|c: char| !c.is_alphanumeric()) + .filter(|word| !word.is_empty()) + .map(str::to_lowercase) +} + +/// The distinct terms of `query`, in the order written, without filler. +pub(super) fn terms(query: &str, noise: &[&str]) -> Vec { + let mut all: Vec = Vec::new(); + for word in words(query) { + if !all.contains(&word) { + all.push(word); + } + } + let meaningful: Vec = all + .iter() + .filter(|term| !FILLER.contains(&term.as_str()) && !noise.contains(&term.as_str())) + .cloned() + .collect(); + if meaningful.is_empty() { + all + } else { + meaningful + } +} + +/// How well `term` matches one field word: 3 for the whole word, 2 for its +/// start, 1 for anywhere inside it (`heatmap` in `complexheatmap`), 0 for no +/// match. A short term matches whole words only. +fn strength(term: &str, word: &str) -> u32 { + if word == term { + 3 + } else if term.chars().count() < MIN_PARTIAL_CHARS { + 0 + } else if word.starts_with(term) { + 2 + } else if word.contains(term) { + 1 + } else { + 0 + } +} + +/// `term`'s strength against `word`, falling back to its singular so +/// `visualizations` finds `visualization` and `heatmaps` finds `heatmap`. +/// `class` and `gis` are left alone. +fn term_strength(term: &str, word: &str) -> u32 { + let direct = strength(term, word); + if direct > 0 { + return direct; + } + match term.strip_suffix('s') { + Some(stem) if stem.chars().count() >= MIN_PARTIAL_CHARS && !stem.ends_with('s') => { + strength(stem, word) + } + _ => 0, + } +} + +/// Rank `entries` against `query`. `fields` names the text of one entry that +/// is searched, and how much a match there counts. +/// +/// An empty (or all-whitespace) query is the browse case: every entry, in +/// registry order. +pub(super) fn rank<'a, T>( + query: &str, + noise: &[&str], + entries: impl IntoIterator, + fields: impl Fn(&'a T) -> Vec<(&'a str, Weight)>, +) -> MarketplaceSearch<'a, T> { + let phrase = query.trim().to_lowercase(); + if phrase.is_empty() { + return MarketplaceSearch { + terms: Vec::new(), + hits: entries + .into_iter() + .map(|entry| MarketplaceSearchHit { + entry, + matched_terms: Vec::new(), + }) + .collect(), + }; + } + let terms = terms(query, noise); + + let mut ranked = Vec::new(); + for entry in entries { + let fields = fields(entry); + let verbatim = fields + .iter() + .any(|(text, _)| text.to_lowercase().contains(&phrase)); + let entry_words: Vec<(String, u32)> = fields + .iter() + .flat_map(|(text, weight)| words(text).map(move |word| (word, *weight as u32))) + .collect(); + + let mut matched_terms = Vec::new(); + let mut score = 0; + for term in &terms { + let best = entry_words + .iter() + .map(|(word, weight)| term_strength(term, word) * weight) + .max() + .unwrap_or(0); + if best > 0 { + matched_terms.push(term.clone()); + score += best; + } + } + if verbatim || !matched_terms.is_empty() { + ranked.push(( + (verbatim, matched_terms.len(), score), + MarketplaceSearchHit { + entry, + matched_terms, + }, + )); + } + } + // Stable, and descending on the key: equal ranks keep registry order. + ranked.sort_by(|(left, _), (right, _)| right.cmp(left)); + MarketplaceSearch { + terms, + hits: ranked.into_iter().map(|(_, hit)| hit).collect(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + struct Entry { + id: &'static str, + name: &'static str, + description: &'static str, + tags: &'static [&'static str], + } + + fn fields(entry: &Entry) -> Vec<(&str, Weight)> { + let mut fields = vec![ + (entry.id, Weight::Name), + (entry.name, Weight::Name), + (entry.description, Weight::Prose), + ]; + fields.extend(entry.tags.iter().map(|tag| (*tag, Weight::Label))); + fields + } + + fn ids<'a>(search: &MarketplaceSearch<'a, Entry>) -> Vec<&'a str> { + search.hits.iter().map(|hit| hit.entry.id).collect() + } + + const ENTRIES: &[Entry] = &[ + Entry { + id: "complex-plots", + name: "Complex Plots", + description: "Draws annotated heat maps with the ComplexHeatmap package.", + tags: &["ComplexHeatmap"], + }, + Entry { + id: "prose-only", + name: "Prose Only", + description: "Mentions scripting in passing.", + tags: &[], + }, + Entry { + id: "r-scripting", + name: "R Scripting", + description: "Tidyverse conventions for R code.", + tags: &["R"], + }, + ]; + + #[test] + fn a_query_is_split_at_whitespace_and_punctuation_and_lowercased() { + assert_eq!( + terms("R scripting, ggplot/Visualization", &[]), + ["r", "scripting", "ggplot", "visualization"] + ); + assert_eq!(terms("r-scripting", &[]), ["r", "scripting"]); + assert_eq!( + terms("ggplot2 ggplot2", &[]), + ["ggplot2"], + "terms are distinct" + ); + } + + #[test] + fn filler_is_dropped_unless_it_is_all_there_is() { + assert_eq!( + terms("a skill about R scripting or ggplot", SKILL_NOISE), + ["r", "scripting", "ggplot"] + ); + assert_eq!(terms("skills", SKILL_NOISE), ["skills"]); + assert_eq!( + terms("skills", EXTENSION_NOISE), + ["skills"], + "a catalog's own noise words are its own" + ); + } + + /// `r` as a substring is in nearly every word of prose; as a term it must + /// mean the R language. + #[test] + fn a_short_term_matches_whole_words_only() { + let search = rank("R scripting", &[], ENTRIES, fields); + let prose = search + .hits + .iter() + .find(|hit| hit.entry.id == "prose-only") + .expect("`scripting` is in its description"); + assert_eq!( + prose.matched_terms, + ["scripting"], + "the `r` inside `scripting` is not the R language" + ); + assert!( + !ids(&search).contains(&"complex-plots"), + "nor is the `r` inside `Draws`" + ); + } + + #[test] + fn a_long_term_matches_inside_a_word_and_a_plural_finds_its_singular() { + assert_eq!( + ids(&rank("heatmap", &[], ENTRIES, fields)), + ["complex-plots"], + "`heatmap` inside `complexheatmap`" + ); + assert_eq!( + ids(&rank("heatmaps", &[], ENTRIES, fields)), + ["complex-plots"], + "no field says `heatmaps`; its singular is inside `complexheatmap`" + ); + assert_eq!( + ids(&rank("scripts", &[], ENTRIES, fields)), + ["r-scripting", "prose-only"], + "`scripts` is not in `scripting`, but `script` starts it" + ); + } + + /// The union, ranked: every entry matching a term is returned, the one + /// matching more terms first, and a name match ahead of a prose match. + #[test] + fn hits_are_the_union_ranked_by_terms_matched_then_by_where() { + let search = rank("R scripting", &[], ENTRIES, fields); + assert_eq!(ids(&search), ["r-scripting", "prose-only"]); + assert_eq!(search.hits[0].matched_terms, ["r", "scripting"]); + assert_eq!(search.hits[1].matched_terms, ["scripting"]); + + let by_place = rank("scripting", &[], ENTRIES, fields); + assert_eq!( + ids(&by_place), + ["r-scripting", "prose-only"], + "a match in the name outranks the same match in the description" + ); + } + + /// Everything the substring matcher found is still found: a query that + /// occurs verbatim in a field is a hit even when its terms are too short + /// to match on their own. + #[test] + fn a_verbatim_occurrence_is_still_a_hit() { + let search = rank("s p", &[], ENTRIES, fields); + assert!(search.is_empty(), "neither `s` nor `p` is a whole word"); + + let search = rank("dy", &[], ENTRIES, fields); + assert_eq!(ids(&search), ["r-scripting"], "`dy` inside `Tidyverse`"); + assert!(search.hits[0].matched_terms.is_empty()); + } + + #[test] + fn an_empty_query_browses_in_registry_order() { + let search = rank(" ", &[], ENTRIES, fields); + assert!(search.terms.is_empty()); + assert_eq!(ids(&search), ["complex-plots", "prose-only", "r-scripting"]); + } +} From fc9741cc4f01d2966c3941375935cd21e859d70c Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 11:15:47 -0700 Subject: [PATCH 2/4] fix(extension-manager): an empty list_resources says who was asked, not "" (F8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `extensionmanager__list_resources` answered `""` when no extension had a resource, so the model could only report that the tool "returned an empty string" — every other empty or refusing result explains itself. An empty listing now says which extensions were asked and that none is publishing anything, how many reachable extensions do not offer resources, and which could not be listed or were refused to this model (a cross-affiliation mismatch). Naming a named extension says it has none. Every name is filtered through Gate E's roster (`allowed_extension_keys`), the rule #219 set for the not-found message next door: a public caller is never named, or counted, a private extension the fan-out declined to reach. --- .../biorouter/src/agents/extension_manager.rs | 466 +++++++++++++++++- 1 file changed, 452 insertions(+), 14 deletions(-) diff --git a/crates/biorouter/src/agents/extension_manager.rs b/crates/biorouter/src/agents/extension_manager.rs index 728269411..ef1e5af86 100644 --- a/crates/biorouter/src/agents/extension_manager.rs +++ b/crates/biorouter/src/agents/extension_manager.rs @@ -170,6 +170,106 @@ fn is_privacy_refusal(err: &ErrorData) -> bool { err.code == ErrorCode::INVALID_REQUEST } +/// What `list_resources` says when an extension the caller NAMED lists nothing +/// (finding F8) — the name is the caller's own, and the reach gate has already +/// admitted it. +fn no_resources_in(extension: &str, supports_resources: bool) -> String { + if supports_resources { + format!( + "`{extension}` has no resources to list: it supports resources but is not \ + publishing any right now." + ) + } else { + format!("`{extension}` has no resources to list: it does not offer resources.") + } +} + +/// Who a `list_resources` fan-out that found nothing may name, partitioned the +/// way its sentence reports them. Every name is one Gate E has already shown +/// the caller. +#[derive(Debug, Default)] +struct UnlistedResources { + /// Support resources, were asked, and had none. + asked: Vec, + /// Support resources and could not be listed. + failed: Vec, + /// Support resources and were refused to this caller's model — a + /// cross-affiliation mismatch (DR-26), which Gate E lists and marks rather + /// than hides. Without this group the sentence would call them extensions + /// that "do not offer resources". + withheld: Vec, + /// How many other reachable extensions do not support resources at all. A + /// count, not a list: the model's own tool list already names them, and + /// the sentence is read to a user. + offer_none: usize, +} + +/// What `list_resources`' fan-out says when no extension listed anything +/// (finding F8). +fn no_resources_listed(unlisted: &UnlistedResources) -> String { + let named = |names: &[String]| { + names + .iter() + .map(|name| format!("`{name}`")) + .collect::>() + .join(", ") + }; + let mut text = String::from("No resources to list."); + match unlisted.asked.as_slice() { + [] => {} + [one] => text.push_str(&format!( + " `{one}` supports resources and was asked, but is not publishing any right now." + )), + many => text.push_str(&format!( + " {} support resources and were asked, but none of them is publishing any right \ + now.", + named(many) + )), + } + match unlisted.failed.as_slice() { + [] => {} + [one] => text.push_str(&format!( + " `{one}` supports resources but could not be listed: its server returned an error." + )), + many => text.push_str(&format!( + " {} support resources but could not be listed: their servers returned an error.", + named(many) + )), + } + match unlisted.withheld.as_slice() { + [] => {} + [one] => text.push_str(&format!( + " `{one}` supports resources, but this chat's model may not read them." + )), + many => text.push_str(&format!( + " {} support resources, but this chat's model may not read them.", + named(many) + )), + } + let offer_none = unlisted.offer_none; + let other = if unlisted.asked.is_empty() + && unlisted.failed.is_empty() + && unlisted.withheld.is_empty() + { + "" + } else { + " other" + }; + match offer_none { + 0 if other.is_empty() => { + text.push_str(" No extension this chat can reach offers resources."); + } + 0 => {} + 1 => text.push_str(&format!( + " The one{other} extension this chat can reach does not offer resources." + )), + n => text.push_str(&format!( + " The{other} {n} extensions this chat can reach do not offer resources." + )), + } + text +} + /// The prefixed tool list and the extension keys that named it, taken together. /// /// Issue #56 Gate E resolves a prefixed tool name against the set of installed @@ -2566,6 +2666,12 @@ impl ExtensionManager { ) }) .map(|lr| { + // Nothing listed is NOTHING, not one empty string: both callers + // compose a sentence from an empty result, and a `""` among the + // fan-out's contents would read as a listing (finding F8). + if lr.resources.is_empty() { + return Vec::new(); + } let resource_list = lr .resources .into_iter() @@ -2599,38 +2705,65 @@ impl ExtensionManager { .await?; // Handle single extension case - self.list_resources_from_extension(extension_name, admitted, cancellation_token) + let listed = self + .list_resources_from_extension(extension_name, admitted, cancellation_token) + .await?; + if !listed.is_empty() { + return Ok(listed); + } + // The gate above admitted this name, and it is the caller's own + // word, so saying it back tells the caller nothing new. + let supports_resources = self + .extensions + .lock() .await + .get(extension_name) + .is_some_and(Extension::supports_resources); + Ok(vec![Content::text(no_resources_in( + extension_name, + supports_resources, + ))]) } None => { // Handle all extensions case using FuturesUnordered let mut futures = FuturesUnordered::new(); - // Create futures for each resource_capable_extension - self.extensions + // The extensions that declare resource support: the ones this + // fan-out asks, and — by difference — the ones it need not. + let capable: Vec = self + .extensions .lock() .await .iter() .filter(|(_name, ext)| ext.supports_resources()) .map(|(name, _ext)| name.clone()) - .for_each(|name| { - let token = cancellation_token.clone(); - futures.push(async move { - self.list_resources_from_extension(&name.clone(), admitted, token) - .await - }); + .collect(); + for name in capable.iter().cloned() { + let token = cancellation_token.clone(); + futures.push(async move { + let listed = self + .list_resources_from_extension(&name, admitted, token) + .await; + (name, listed) }); + } let mut all_resources = Vec::new(); + let mut asked = Vec::new(); + let mut failed = Vec::new(); let mut errors = Vec::new(); // Process results as they complete - while let Some(result) = futures.next().await { + while let Some((name, result)) = futures.next().await { match result { Ok(content) => { + asked.push(name); all_resources.extend(content); } Err(tool_error) => { + if !is_privacy_refusal(&tool_error) { + failed.push(name); + } errors.push(tool_error); } } @@ -2663,7 +2796,54 @@ impl ExtensionManager { ); } - Ok(all_resources) + if !all_resources.is_empty() { + return Ok(all_resources); + } + + // Finding F8 (2026-09-10 composer QA run): an empty listing + // used to come back as `""`, and the model could only report + // that the tool "returned an empty string". Every other empty + // or refusing result explains itself, so this one says which + // extensions were asked and that none of them had anything. + // + // ⚠ **Every name here is filtered through Gate E's roster**, + // the rule #219 set for the not-found message in + // `read_resource_tool`, and for the same reason: this sentence + // is composed right after a loop that declined to reach the + // private extensions, so naming what the loop consulted — or, + // worse, what it skipped — would hand a public caller the + // private roster Gate E withholds. A refused extension is never + // named and never counted. `admitted` is threaded, never + // resampled, and the roster is sorted because the map's + // iteration order is randomised per process. + let roster = self.allowed_extension_keys(admitted).await; + let shown = |mut names: Vec| { + names.retain(|name| roster.contains(name)); + names.sort(); + names + }; + // What is left of the capable set once the asked and the failed + // are taken out is what the loop refused. Of that, only the + // part Gate E shows this caller is named — the cross- + // affiliation mismatches DR-26 lists and marks — and the rest, + // the private extensions a public caller may not see, vanishes + // here exactly as it vanished from the tool list. + let withheld = shown( + capable + .iter() + .filter(|name| !asked.contains(name) && !failed.contains(name)) + .cloned() + .collect(), + ); + let offer_none = roster.iter().filter(|name| !capable.contains(name)).count(); + Ok(vec![Content::text(no_resources_listed( + &UnlistedResources { + asked: shown(asked), + failed: shown(failed), + withheld, + offer_none, + }, + ))]) } } } @@ -6953,6 +7133,9 @@ mod tests { struct CountingClient { label: &'static str, calls: Arc, + /// Whether `list_resources` returns a resource. `false` is a server + /// that supports resources and is publishing none — finding F8's case. + publishes: bool, } impl CountingClient { @@ -6960,6 +7143,14 @@ mod tests { Self { label, calls: Arc::new(std::sync::atomic::AtomicUsize::new(0)), + publishes: true, + } + } + + fn publishing_nothing(label: &'static str) -> Self { + Self { + publishes: false, + ..Self::new(label) } } @@ -6990,12 +7181,17 @@ mod tests { ) -> Result { use rmcp::model::AnnotateAble; self.hit(); - Ok(ListResourcesResult { - resources: vec![rmcp::model::RawResource::new( + let resources = if self.publishes { + vec![rmcp::model::RawResource::new( "res://x", format!("{}-resource", self.sentinel()), ) - .no_annotation()], + .no_annotation()] + } else { + vec![] + }; + Ok(ListResourcesResult { + resources, next_cursor: None, meta: None, }) @@ -7404,6 +7600,248 @@ mod tests { ))); } + /// Finding F8's shape: extensions that SUPPORT resources and publish none + /// — the private `ucsfomopagent` and the public `developer` — beside the + /// public `todo`, which declares no resource support at all. The QA + /// sandbox's `computercontroller` had cached nothing, which is the + /// `developer` row; the private row is added so the roster filter can be + /// seen biting rather than assumed. + async fn quiet_resources_fixture() -> (TempDir, ExtensionManager, CountingClient) { + let dir = tempfile::tempdir().unwrap(); + let session_manager = Arc::new(crate::session::SessionManager::new( + dir.path().to_path_buf(), + )); + let em = ExtensionManager::new( + Arc::new(Mutex::new(Some(provider_at( + crate::privacy::ProviderTier::Public, + )))), + session_manager, + ); + let info = ServerInfo { + capabilities: rmcp::model::ServerCapabilities::builder() + .enable_tools() + .enable_resources() + .build(), + ..Default::default() + }; + let private = CountingClient::publishing_nothing("private"); + for (name, client) in [ + ("ucsfomopagent", private.clone()), + ("developer", CountingClient::publishing_nothing("public")), + ] { + em.add_client( + normalize(name), + ExtensionConfig::Builtin { + name: name.to_string(), + display_name: Some(name.to_string()), + description: "built-in".to_string(), + timeout: None, + bundled: None, + available_tools: vec![], + }, + Arc::new(client), + Some(info.clone()), + None, + ) + .await; + } + // No `ServerInfo`, so no declared resource support. + em.add_mock_extension("todo".to_string(), Arc::new(MockClient {})) + .await; + (dir, em, private) + } + + /// `list_resources` as the `extensionmanager__list_resources` tool calls + /// it — an admitted capability, never a fresh sample — rendered to the text + /// the model reads. + async fn listed_resources( + em: &ExtensionManager, + params: Value, + caller: crate::privacy::ProviderTier, + ) -> String { + em.list_resources( + params, + Some(crate::privacy::CallCapability::for_test(caller, true)), + CancellationToken::default(), + ) + .await + .expect("an empty listing is an answer, not an error") + .iter() + .filter_map(|c| c.as_text().map(|t| t.text.clone())) + .collect::>() + .join("\n") + } + + /// Finding F8: `list_resources` found nothing and answered `""`, so the + /// model could only report that the tool "returned an empty string". It now + /// says who was asked and that none of them had anything. + /// + /// ⚠ **Both columns are needed, as in #219's M6 test next door.** The public + /// sentence must not name `ucsfomopagent` — it is composed right after a + /// loop that declined to reach it, and naming what the loop skipped is the + /// leak #219 closed in `read_resource_tool`. And it must still name + /// `developer`, or a sentence that named nothing would pass. The private + /// column is what shows the name was FILTERED rather than never collected. + #[tokio::test] + async fn an_empty_resource_listing_says_who_was_asked_naming_only_gate_es_roster() { + let (_dir, em, private) = quiet_resources_fixture().await; + + let as_public = listed_resources( + &em, + serde_json::json!({}), + crate::privacy::ProviderTier::Public, + ) + .await; + assert_eq!( + as_public, + "No resources to list. `developer` supports resources and was asked, but is not \ + publishing any right now. The one other extension this chat can reach does not \ + offer resources." + ); + assert!(!as_public.contains("ucsfomopagent"), "{as_public}"); + assert_eq!( + private.contacted(), + 0, + "the public listing asked the private server" + ); + + let as_private = listed_resources( + &em, + serde_json::json!({}), + crate::privacy::ProviderTier::Private, + ) + .await; + assert_eq!( + as_private, + "No resources to list. `developer`, `ucsfomopagent` support resources and were \ + asked, but none of them is publishing any right now. The one other extension this \ + chat can reach does not offer resources." + ); + assert_eq!(private.contacted(), 1); + } + + /// The named branch says the named extension has none — and a public caller + /// naming a private extension is still REFUSED, in the words a name that is + /// not installed gets (finding M18). "`ucsfomopagent` has no resources" + /// would confirm to a public caller that it is installed. + #[tokio::test] + async fn a_named_extension_with_nothing_to_list_says_so() { + use crate::privacy::ProviderTier::{Private, Public}; + let (_dir, em, private) = quiet_resources_fixture().await; + + assert_eq!( + listed_resources(&em, serde_json::json!({ "extension": "developer" }), Public).await, + "`developer` has no resources to list: it supports resources but is not publishing \ + any right now." + ); + assert_eq!( + listed_resources( + &em, + serde_json::json!({ "extension": "ucsfomopagent" }), + Private + ) + .await, + "`ucsfomopagent` has no resources to list: it supports resources but is not \ + publishing any right now." + ); + assert_eq!(private.contacted(), 1, "only the private caller reached it"); + + let refused = |name: &'static str| { + let em = &em; + async move { + em.list_resources( + serde_json::json!({ "extension": name }), + Some(crate::privacy::CallCapability::for_test(Public, true)), + CancellationToken::default(), + ) + .await + .expect_err("a public caller reaches neither") + .message + .to_string() + } + }; + assert_eq!( + refused("ucsfomopagent") + .await + .replace("ucsfomopagent", "NAME"), + refused("nonexistent_ext") + .await + .replace("nonexistent_ext", "NAME"), + ); + assert_eq!(private.contacted(), 1); + } + + /// A model bound to ANOTHER institution is shown `ucsfomopagent` — DR-26 + /// lists and marks a mismatch rather than hiding it — and refused its + /// resources. The sentence must say that, not count it among the + /// extensions that "do not offer resources", which would be false. + #[tokio::test] + async fn an_empty_listing_names_a_cross_affiliation_refusal_as_withheld() { + let (_dir, em, private) = quiet_resources_fixture().await; + let elsewhere = crate::privacy::CallCapability::for_test_affiliated( + crate::privacy::ProviderTier::Private, + true, + Some(crate::privacy::affiliation::ModelAffiliation::institution( + crate::privacy::affiliation::InstitutionId::new("stanford"), + )), + ); + let text = em + .list_resources( + serde_json::json!({}), + Some(elsewhere), + CancellationToken::default(), + ) + .await + .expect("an empty listing is an answer, not an error") + .iter() + .filter_map(|c| c.as_text().map(|t| t.text.clone())) + .collect::>() + .join("\n"); + assert_eq!( + text, + "No resources to list. `developer` supports resources and was asked, but is not \ + publishing any right now. `ucsfomopagent` supports resources, but this chat's model \ + may not read them. The one other extension this chat can reach does not offer \ + resources." + ); + assert_eq!( + private.contacted(), + 0, + "Gate C refused it before any contact" + ); + } + + /// The composer's other branches, which the fixtures above do not reach: + /// no reachable extension supports resources at all, and ones that could + /// not be listed. + #[test] + fn the_empty_listing_sentence_accounts_for_every_group_it_is_given() { + let names = |names: &[&str]| names.iter().map(|n| n.to_string()).collect::>(); + assert_eq!( + no_resources_listed(&UnlistedResources::default()), + "No resources to list. No extension this chat can reach offers resources." + ); + assert_eq!( + no_resources_listed(&UnlistedResources { + offer_none: 11, + ..Default::default() + }), + "No resources to list. The 11 extensions this chat can reach do not offer resources." + ); + assert_eq!( + no_resources_listed(&UnlistedResources { + failed: names(&["files", "notes"]), + withheld: names(&["cdwagent"]), + offer_none: 2, + ..Default::default() + }), + "No resources to list. `files`, `notes` support resources but could not be listed: \ + their servers returned an error. `cdwagent` supports resources, but this chat's \ + model may not read them. The other 2 extensions this chat can reach do not offer \ + resources." + ); + } + /// An MCP prompt body is server-authored text that lands in the transcript /// verbatim, so a refusal that echoed it would defeat the point of refusing. #[tokio::test] From 128b91384abb80ac43a9692b151ab25d060a749b Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 11:15:47 -0700 Subject: [PATCH 3/4] docs(extension-manager): the marketplace search ranks a phrase; an empty resource listing explains itself --- docs/extensions/built-in/extension-manager.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/extensions/built-in/extension-manager.md b/docs/extensions/built-in/extension-manager.md index ba0df2358..ea7581ff6 100644 --- a/docs/extensions/built-in/extension-manager.md +++ b/docs/extensions/built-in/extension-manager.md @@ -62,12 +62,12 @@ The result is a more focused session where BioRouter has exactly the tools it ne | Tool | Description | Use Case | |------|-------------|----------| | `search_available_extensions` | List installed third-party extensions and exact names | Finding what is already installed | -| `search_marketplace_extensions` | Browse or search trusted BAAM entries visible to this model — omit the query to list everything | Finding a package to install | +| `search_marketplace_extensions` | Browse or search trusted BAAM entries visible to this model — omit the query to list everything. A query is matched word by word, so a phrase returns every entry matching any of its words, best match first; a query that matches nothing says so and suggests shorter terms | Finding a package to install | | `manage_extensions` | Enable or disable an extension by name | Loading/unloading extensions dynamically | | `install_extension` | Install a BAAM marketplace extension end to end | The extension is not installed at all | | `delete_extension_package` | Delete one or up to 50 validated marketplace packages after approval | Permanent removal of something installed from BAAM; shared credentials are retained | | `remove_extension` | Remove one or up to 50 installed extensions by installed name after approval | Permanent removal of anything else — a sideloaded `.brxt`, a hand-configured MCP server | -| `list_resources` | List resources from extensions (if supported) | Discovering available data sources | +| `list_resources` | List resources from extensions (if supported). When there are none, it says which extensions were asked — naming only extensions the model has already been shown | Discovering available data sources | | `read_resource` | Read specific resource content (if supported) | Accessing extension-provided data | > **Tip.** Not every tool in this table is offered in every session. The resource tools (`list_resources` and `read_resource`) appear only when at least one enabled extension supports resources. `install_extension`, `delete_extension_package` and `remove_extension` each wait on your approval, so they are withheld entirely where no one can be asked for it — on a daemon started by `biorouter serve` for browser access, for instance. Browsing and searching are read-only and always available. From 0b438cc5738740c50c0770175b10fac4bb5a2796 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 11:26:40 -0700 Subject: [PATCH 4/4] refactor(extension-manager): compose the empty-listing roster in its own method list_resources had grown to 105 lines, over the too_many_lines baseline. The Gate E filtering moves into unlisted_resources unchanged. --- .../biorouter/src/agents/extension_manager.rs | 100 ++++++++++-------- 1 file changed, 55 insertions(+), 45 deletions(-) diff --git a/crates/biorouter/src/agents/extension_manager.rs b/crates/biorouter/src/agents/extension_manager.rs index ef1e5af86..cbb6d51c8 100644 --- a/crates/biorouter/src/agents/extension_manager.rs +++ b/crates/biorouter/src/agents/extension_manager.rs @@ -2799,55 +2799,65 @@ impl ExtensionManager { if !all_resources.is_empty() { return Ok(all_resources); } - - // Finding F8 (2026-09-10 composer QA run): an empty listing - // used to come back as `""`, and the model could only report - // that the tool "returned an empty string". Every other empty - // or refusing result explains itself, so this one says which - // extensions were asked and that none of them had anything. - // - // ⚠ **Every name here is filtered through Gate E's roster**, - // the rule #219 set for the not-found message in - // `read_resource_tool`, and for the same reason: this sentence - // is composed right after a loop that declined to reach the - // private extensions, so naming what the loop consulted — or, - // worse, what it skipped — would hand a public caller the - // private roster Gate E withholds. A refused extension is never - // named and never counted. `admitted` is threaded, never - // resampled, and the roster is sorted because the map's - // iteration order is randomised per process. - let roster = self.allowed_extension_keys(admitted).await; - let shown = |mut names: Vec| { - names.retain(|name| roster.contains(name)); - names.sort(); - names - }; - // What is left of the capable set once the asked and the failed - // are taken out is what the loop refused. Of that, only the - // part Gate E shows this caller is named — the cross- - // affiliation mismatches DR-26 lists and marks — and the rest, - // the private extensions a public caller may not see, vanishes - // here exactly as it vanished from the tool list. - let withheld = shown( - capable - .iter() - .filter(|name| !asked.contains(name) && !failed.contains(name)) - .cloned() - .collect(), - ); - let offer_none = roster.iter().filter(|name| !capable.contains(name)).count(); - Ok(vec![Content::text(no_resources_listed( - &UnlistedResources { - asked: shown(asked), - failed: shown(failed), - withheld, - offer_none, - }, - ))]) + let unlisted = self + .unlisted_resources(&capable, asked, failed, admitted) + .await; + Ok(vec![Content::text(no_resources_listed(&unlisted))]) } } } + /// Who an empty `list_resources` fan-out may name, and how (finding F8, + /// 2026-09-10 composer QA run). An empty listing used to come back as + /// `""`, and the model could only report that the tool "returned an empty + /// string"; every other empty or refusing result explains itself. + /// + /// `capable` is every extension that declares resource support; `asked` + /// and `failed` are the ones the fan-out listed and could not list. + /// + /// ⚠ **Every name is filtered through Gate E's roster**, the rule #219 set + /// for the not-found message in `read_resource_tool`, and for the same + /// reason: this is composed right after a loop that declined to reach the + /// private extensions, so naming what the loop consulted — or, worse, what + /// it skipped — would hand a public caller the private roster Gate E + /// withholds. A refused extension is never named and never counted. + /// `admitted` is threaded, never resampled, and names are sorted because + /// the map's iteration order is randomised per process. + async fn unlisted_resources( + &self, + capable: &[String], + asked: Vec, + failed: Vec, + admitted: Option, + ) -> UnlistedResources { + let roster = self.allowed_extension_keys(admitted).await; + let shown = |mut names: Vec| { + names.retain(|name| roster.contains(name)); + names.sort(); + names + }; + // What is left of the capable set once the asked and the failed are + // taken out is what the loop refused. Of that, only the part Gate E + // shows this caller is named — the cross-affiliation mismatches DR-26 + // lists and marks — and the rest, the private extensions a public + // caller may not see, vanishes here exactly as it vanished from the + // tool list. + let withheld = shown( + capable + .iter() + .filter(|name| !asked.contains(name) && !failed.contains(name)) + .cloned() + .collect(), + ); + let offer_none = roster.iter().filter(|name| !capable.contains(name)).count(); + UnlistedResources { + asked: shown(asked), + failed: shown(failed), + withheld, + offer_none, + } + } + async fn prefixed_tool_name(&self, tool_name: &str) -> String { if !tool_name.contains("__") && ["execute_code", "read_module", "search_modules"].contains(&tool_name)