refactor(tools): auto-compact WASM tool schemas, add descriptions, improve credential prompts (#1525)

* fix(tools): add missing description, parameters, and improve credential prompts

Silence three categories of startup warnings emitted by
CapabilitiesFile::validate() and WasmToolLoader:

1. "description" field missing → add tool descriptions to all manifests
2. "parameters" field missing → add action-enum parameter schemas
3. Short credential prompts (<30 chars) → append source URLs

Affects: github, gmail, google-calendar, google-docs, google-drive,
google-sheets, google-slides, slack, telegram, llm-context, feishu.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* refactor(tools): auto-compact WASM tool schemas from module exports

Replace the manual `parameters` field in capabilities JSON with automatic
schema compaction. WasmToolSchemas::compact_schema() derives a compact
advertised schema from the WASM module's schema() export by keeping only
required and enum-constrained properties. The full schema remains
available via tool_info(detail: "schema").

This eliminates:
- The `parameters` field from CapabilitiesFile and all 11 sidecar JSONs
- The "missing parameters" startup warning from the loader
- Manual maintenance of duplicate schema data

The `description` field in capabilities JSON is retained.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix(tests): remove cap_file.parameters reference in test_rig

The parameters field was removed from CapabilitiesFile in the previous
commit. Update test_rig.rs to match — schema is now auto-compacted from
the WASM module export, no sidecar override needed.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix(tools): handle oneOf schemas in compact_schema, add tool name to warning

Address PR review feedback:
- compact_schema now collects properties from oneOf/anyOf/allOf variants,
  fixing GitHub-style schemas that have no top-level properties
- Use HashSet for required lookup instead of Vec::contains
- Add tool name to "Capabilities file not found" warning for consistency

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix(tools): merge oneOf const values into enum, cap property collection

Address review feedback from @serrrfirat:

1. Merge const values across oneOf variants into a single enum array,
   so the LLM sees all valid actions (not just the first variant's const).
2. Cap property collection at 100 to bound allocations.
3. Also keep properties with const constraint (single-variant case).
4. Update doc comment to describe variant collection and design choices
   around variant-level required fields.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
Illia Polosukhin
2026-03-23 21:59:14 -07:00
committed by GitHub
co-authored by Claude Opus 4.6
parent b441ebec02
commit 3fdb187796
16 changed files with 322 additions and 256 deletions
+20 -102
View File
@@ -47,12 +47,6 @@ pub struct CapabilitiesFile {
#[serde(default)]
pub description: Option<String>,
/// JSON Schema for the tool's input parameters.
/// Used as the `Tool::parameters_schema()` return value.
/// If omitted, a permissive fallback is used (with a warning).
#[serde(default)]
pub parameters: Option<serde_json::Value>,
/// Extension version (semver).
#[serde(default)]
pub version: Option<String>,
@@ -103,9 +97,6 @@ pub struct CapabilitiesFile {
/// Maximum length for the description field to prevent memory abuse.
const MAX_DESCRIPTION_CHARS: usize = 4096;
/// Maximum serialized size of the parameters schema JSON.
const MAX_PARAMETERS_SCHEMA_BYTES: usize = 64 * 1024;
impl CapabilitiesFile {
/// Parse from JSON string.
pub fn from_json(json: &str) -> Result<Self, serde_json::Error> {
@@ -135,18 +126,6 @@ impl CapabilitiesFile {
);
self.description = Some(truncated.to_string());
}
// Drop oversized parameters schema (issue #977)
if let Some(ref params) = self.parameters {
let size = params.to_string().len();
if size > MAX_PARAMETERS_SCHEMA_BYTES {
tracing::warn!(
"Capabilities parameters schema dropped ({} bytes exceeds {} limit)",
size,
MAX_PARAMETERS_SCHEMA_BYTES,
);
self.parameters = None;
}
}
}
/// Merge nested `capabilities` wrapper into top-level fields.
@@ -171,7 +150,6 @@ impl CapabilitiesFile {
if let Some(inner) = self.capabilities.take() {
let inner = inner.resolve_nested_inner(depth + 1);
self.description = self.description.or(inner.description);
self.parameters = self.parameters.or(inner.parameters);
self.http = self.http.or(inner.http);
self.secrets = self.secrets.or(inner.secrets);
self.tool_invoke = self.tool_invoke.or(inner.tool_invoke);
@@ -1424,26 +1402,12 @@ mod tests {
);
}
// ── Tool description and parameters schema ──────────────────────────
// ── Tool description ────────────────────────────────────────────────
#[test]
fn test_parse_description_and_parameters() {
fn test_parse_description() {
let json = r#"{
"description": "Search the web using Brave Search API",
"parameters": {
"type": "object",
"properties": {
"query": {
"type": "string",
"description": "Search query"
},
"count": {
"type": "integer",
"description": "Number of results"
}
},
"required": ["query"]
}
"description": "Search the web using Brave Search API"
}"#;
let caps = CapabilitiesFile::from_json(json).unwrap();
@@ -1451,28 +1415,10 @@ mod tests {
caps.description.as_deref(),
Some("Search the web using Brave Search API")
);
let params = caps.parameters.unwrap();
assert_eq!(params["type"], "object");
assert!(params["properties"]["query"].is_object());
assert_eq!(params["required"][0], "query");
}
#[test]
fn test_parse_description_only() {
let json = r#"{
"description": "A tool without explicit parameters schema"
}"#;
let caps = CapabilitiesFile::from_json(json).unwrap();
assert_eq!(
caps.description.as_deref(),
Some("A tool without explicit parameters schema")
);
assert!(caps.parameters.is_none());
}
#[test]
fn test_parse_without_description_or_parameters() {
fn test_parse_without_description() {
let json = r#"{
"http": {
"allowlist": [{ "host": "api.example.com" }]
@@ -1484,24 +1430,28 @@ mod tests {
caps.description.is_none(),
"description should be None when not provided"
);
assert!(
caps.parameters.is_none(),
"parameters should be None when not provided"
);
}
#[test]
fn test_parameters_field_silently_ignored() {
// Backward compat: old capabilities files with "parameters" still parse.
let json = r#"{
"description": "A tool",
"parameters": {
"type": "object",
"properties": { "action": { "type": "string" } }
}
}"#;
let caps = CapabilitiesFile::from_json(json).unwrap();
assert_eq!(caps.description.as_deref(), Some("A tool"));
}
#[test]
fn test_resolve_nested_description_promoted() {
let json = r#"{
"capabilities": {
"description": "Inner tool description",
"parameters": {
"type": "object",
"properties": {
"input": { "type": "string" }
},
"required": ["input"]
}
"description": "Inner tool description"
}
}"#;
@@ -1511,10 +1461,6 @@ mod tests {
Some("Inner tool description"),
"description should be promoted from inner capabilities"
);
assert!(
caps.parameters.is_some(),
"parameters should be promoted from inner capabilities"
);
}
#[test]
@@ -1564,32 +1510,4 @@ mod tests {
desc.len()
);
}
/// Regression test for issue #977: oversized parameters schema is dropped.
#[test]
fn test_oversized_parameters_schema_dropped() {
// Build a parameters schema larger than MAX_PARAMETERS_SCHEMA_BYTES
let mut properties = serde_json::Map::new();
for i in 0..2000 {
properties.insert(
format!("field_{i}"),
serde_json::json!({
"type": "string",
"description": "x".repeat(50)
}),
);
}
let schema = serde_json::json!({
"type": "object",
"properties": properties,
});
let json = serde_json::json!({
"parameters": schema,
});
let caps = CapabilitiesFile::from_json(&json.to_string()).unwrap();
assert!(
caps.parameters.is_none(),
"oversized parameters schema should be dropped"
);
}
}
+36 -58
View File
@@ -123,73 +123,51 @@ impl WasmToolLoader {
}
let wasm_bytes = fs::read(wasm_path).await?;
// Read capabilities (optional) and extract OAuth refresh config,
// tool description, and parameter schema.
let (capabilities, oauth_refresh, description, schema) =
if let Some(cap_path) = capabilities_path {
if cap_path.exists() {
let cap_bytes = fs::read(cap_path).await?;
let cap_file = CapabilitiesFile::from_bytes(&cap_bytes)
.map_err(|e| WasmLoadError::InvalidCapabilities(e.to_string()))?;
cap_file.validate(name);
// Read capabilities (optional) and extract OAuth refresh config
// and tool description. Parameter schema is auto-derived from the
// WASM module's schema() export (see WasmToolSchemas::compact_schema).
let (capabilities, oauth_refresh, description) = if let Some(cap_path) = capabilities_path {
if cap_path.exists() {
let cap_bytes = fs::read(cap_path).await?;
let cap_file = CapabilitiesFile::from_bytes(&cap_bytes)
.map_err(|e| WasmLoadError::InvalidCapabilities(e.to_string()))?;
cap_file.validate(name);
// Check WIT version compatibility
check_wit_version_compat(
name,
cap_file.wit_version.as_deref(),
crate::tools::wasm::WIT_TOOL_VERSION,
)?;
// Check WIT version compatibility
check_wit_version_compat(
name,
cap_file.wit_version.as_deref(),
crate::tools::wasm::WIT_TOOL_VERSION,
)?;
let caps = cap_file.to_capabilities();
let oauth = resolve_oauth_refresh_config(&cap_file);
let desc = cap_file.description.clone();
// Validate parameters schema before accepting it.
let params = cap_file.parameters.clone().and_then(|p| {
let errors = crate::tools::validate_tool_schema(&p, name);
if errors.is_empty() {
Some(p)
} else {
tracing::warn!(
tool = name,
?errors,
"Invalid parameters schema in capabilities.json, \
using permissive fallback"
);
None
}
});
if desc.is_none() {
tracing::warn!(
tool = name,
path = %cap_path.display(),
"Capabilities file missing \"description\" field; \
tool will use generic fallback description"
);
}
if params.is_none() && cap_file.parameters.is_none() {
tracing::warn!(
tool = name,
path = %cap_path.display(),
"Capabilities file missing \"parameters\" field; \
tool will accept any JSON object (permissive fallback)"
);
}
(caps, oauth, desc, params)
} else {
let caps = cap_file.to_capabilities();
let oauth = resolve_oauth_refresh_config(&cap_file);
let desc = cap_file.description.clone();
if desc.is_none() {
tracing::warn!(
tool = name,
path = %cap_path.display(),
"Capabilities file not found, using default (no permissions)"
"Capabilities file missing \"description\" field; \
tool will use generic fallback description"
);
(Capabilities::default(), None, None, None)
}
(caps, oauth, desc)
} else {
tracing::warn!(
tool = name,
"No capabilities file for WASM tool; \
tool will use generic fallback description and accept any JSON object"
path = %cap_path.display(),
"Capabilities file not found, using default (no permissions)"
);
(Capabilities::default(), None, None, None)
};
(Capabilities::default(), None, None)
}
} else {
tracing::warn!(
tool = name,
"No capabilities file for WASM tool; \
tool will use generic fallback description"
);
(Capabilities::default(), None, None)
};
// Register the tool
self.registry
@@ -200,7 +178,7 @@ impl WasmToolLoader {
capabilities,
limits: None,
description: description.as_deref(),
schema,
schema: None,
secrets_store: self.secrets_store.clone(),
oauth_refresh,
})
+243 -42
View File
@@ -656,12 +656,125 @@ impl WasmToolSchemas {
}
fn new(discovery: serde_json::Value) -> Self {
let advertised = Self::compact_schema(&discovery);
Self {
advertised: Self::permissive_schema(),
advertised,
discovery,
}
}
/// Derive a compact advertised schema from the full discovery schema.
///
/// Collects properties from top-level `properties` and from
/// `oneOf`/`anyOf`/`allOf` variants. Keeps only properties that are in
/// the top-level `required` array or carry an `enum`/`const` constraint.
/// For properties defined via `const` across multiple variants (e.g.
/// `"action": {"const": "get_repo"}` in each `oneOf` branch), the `const`
/// values are merged into a single `enum` array.
///
/// Variant-level `required` fields (e.g. `owner`, `repo` required within
/// each `oneOf` variant but not top-level) are intentionally omitted from
/// the compact schema — the LLM can discover them via
/// `tool_info(detail: "schema")`.
///
/// At most `MAX_COMPACT_PROPERTIES` properties are collected to bound
/// allocations from adversarial schemas.
fn compact_schema(discovery: &serde_json::Value) -> serde_json::Value {
const MAX_COMPACT_PROPERTIES: usize = 100;
let required: std::collections::HashSet<String> = discovery
.get("required")
.and_then(|r| r.as_array())
.map(|arr| {
arr.iter()
.filter_map(|v| v.as_str().map(String::from))
.collect()
})
.unwrap_or_default();
// Collect properties from top-level and oneOf/anyOf/allOf variants.
// For properties with `const` across variants, merge into an `enum`.
let mut all_properties = serde_json::Map::new();
// Track const values per property to merge into enum.
let mut const_values: std::collections::HashMap<String, Vec<serde_json::Value>> =
std::collections::HashMap::new();
if let Some(props) = discovery.get("properties").and_then(|p| p.as_object()) {
for (k, v) in props {
if all_properties.len() >= MAX_COMPACT_PROPERTIES {
break;
}
all_properties.insert(k.clone(), v.clone());
}
}
for key in ["oneOf", "anyOf", "allOf"] {
if let Some(variants) = discovery.get(key).and_then(|v| v.as_array()) {
for variant in variants {
if let Some(props) = variant.get("properties").and_then(|p| p.as_object()) {
for (k, v) in props {
if all_properties.len() >= MAX_COMPACT_PROPERTIES
&& !all_properties.contains_key(k)
{
continue;
}
// Track const values for merging into enum.
if let Some(c) = v.get("const") {
const_values.entry(k.clone()).or_default().push(c.clone());
}
all_properties.entry(k.clone()).or_insert_with(|| v.clone());
}
}
}
}
}
// Merge collected const values into enum arrays.
for (name, values) in &const_values {
if values.len() > 1
&& let Some(prop) = all_properties.get_mut(name)
{
let mut merged = prop.clone();
if let Some(obj) = merged.as_object_mut() {
obj.remove("const");
obj.insert("enum".to_string(), serde_json::Value::Array(values.clone()));
}
*prop = merged;
}
}
if all_properties.is_empty() {
return Self::permissive_schema();
}
let kept: serde_json::Map<String, serde_json::Value> = all_properties
.into_iter()
.filter(|(name, prop)| {
required.contains(name) || prop.get("enum").is_some() || prop.get("const").is_some()
})
.collect();
if kept.is_empty() {
return Self::permissive_schema();
}
let kept_required: Vec<serde_json::Value> = required
.iter()
.filter(|name| kept.contains_key(name.as_str()))
.map(|name| serde_json::Value::String(name.clone()))
.collect();
let mut result = serde_json::json!({
"type": "object",
"properties": kept,
"additionalProperties": true,
});
if !kept_required.is_empty() {
result["required"] = serde_json::Value::Array(kept_required);
}
result
}
fn with_override(&self, schema: serde_json::Value) -> Self {
Self {
advertised: schema.clone(),
@@ -1655,7 +1768,7 @@ mod tests {
}
#[tokio::test]
async fn test_advertised_schema_stays_permissive_until_sidecar_override() {
async fn test_advertised_schema_auto_compacted_from_discovery() {
let discovery_schema = serde_json::json!({
"type": "object",
"properties": {
@@ -1675,42 +1788,7 @@ mod tests {
wrapper.schemas = super::WasmToolSchemas::new(discovery_schema.clone());
wrapper.description = "Search documents".to_string();
// Advertised schema stays permissive; discovery holds the typed schema
assert_eq!(
wrapper.parameters_schema(),
serde_json::json!({
"type": "object",
"properties": {},
"additionalProperties": true
})
);
assert_eq!(wrapper.discovery_schema(), discovery_schema);
// Raw description is clean — no tool_info hint baked in
assert!(!wrapper.description().contains("tool_info"));
// But schema() composes the hint at display time when advertised is permissive
let schema = wrapper.schema();
assert!(
schema.description.contains("tool_info"),
"schema().description should contain tool_info hint: {}",
schema.description
);
assert!(
schema.description.contains("include_schema: true"),
"hint should mention include_schema: true: {}",
schema.description
);
// After sidecar override, both schemas match and hint disappears
let wrapper = wrapper.with_schema(serde_json::json!({
"type": "object",
"properties": {
"query": { "type": "string" }
},
"required": ["query"]
}));
// Advertised schema is auto-compacted: keeps required props, drops optional
assert_eq!(
wrapper.parameters_schema(),
serde_json::json!({
@@ -1718,20 +1796,143 @@ mod tests {
"properties": {
"query": { "type": "string" }
},
"required": ["query"]
"required": ["query"],
"additionalProperties": true
})
);
assert_eq!(wrapper.discovery_schema(), wrapper.parameters_schema());
// Discovery retains the full schema
assert_eq!(wrapper.discovery_schema(), discovery_schema);
// With typed schema, schema() should NOT include tool_info hint
// Compacted schema has typed properties, so no tool_info hint needed
let schema = wrapper.schema();
assert!(
!schema.description.contains("tool_info"),
"schema().description should not contain tool_info hint when typed: {}",
"schema().description should not contain tool_info hint when auto-compacted: {}",
schema.description
);
}
#[test]
fn test_compact_schema_keeps_required_and_enum_properties() {
let schema = serde_json::json!({
"type": "object",
"properties": {
"action": {
"type": "string",
"enum": ["list", "get", "create"],
"description": "The operation"
},
"query": { "type": "string" },
"limit": { "type": "integer" },
"format": {
"type": "string",
"enum": ["json", "csv"]
}
},
"required": ["action"]
});
let compacted = super::WasmToolSchemas::compact_schema(&schema);
let props = compacted["properties"].as_object().unwrap();
// action: required + enum → kept
assert!(props.contains_key("action"));
// format: has enum → kept
assert!(props.contains_key("format"));
// query: not required, no enum → dropped
assert!(!props.contains_key("query"));
// limit: not required, no enum → dropped
assert!(!props.contains_key("limit"));
// additionalProperties lets the LLM still pass dropped props
assert_eq!(compacted["additionalProperties"], true);
assert_eq!(compacted["required"], serde_json::json!(["action"]));
}
#[test]
fn test_compact_schema_falls_back_to_permissive_when_empty() {
// No required, no enum → permissive fallback
let schema = serde_json::json!({
"type": "object",
"properties": {
"query": { "type": "string" },
"limit": { "type": "integer" }
}
});
let compacted = super::WasmToolSchemas::compact_schema(&schema);
assert!(compacted["properties"].as_object().unwrap().is_empty());
}
#[test]
fn test_compact_schema_handles_no_properties() {
let schema = serde_json::json!({ "type": "object" });
let compacted = super::WasmToolSchemas::compact_schema(&schema);
assert!(compacted["properties"].as_object().unwrap().is_empty());
}
#[test]
fn test_compact_schema_handles_oneof_variants() {
// GitHub-style schema: oneOf with no top-level properties, const per variant
let schema = serde_json::json!({
"type": "object",
"required": ["action"],
"oneOf": [
{
"properties": {
"action": { "const": "get_repo" },
"owner": { "type": "string" },
"repo": { "type": "string" }
},
"required": ["action", "owner", "repo"]
},
{
"properties": {
"action": { "const": "list_issues" },
"owner": { "type": "string" },
"repo": { "type": "string" },
"state": { "type": "string", "enum": ["open", "closed", "all"] }
},
"required": ["action", "owner", "repo"]
}
]
});
let compacted = super::WasmToolSchemas::compact_schema(&schema);
let props = compacted["properties"].as_object().unwrap();
// action: required + const values merged into enum → kept
let action = &props["action"];
assert!(
action.get("enum").is_some(),
"action const values should be merged into enum: {action}"
);
let action_enum = action["enum"].as_array().unwrap();
assert!(
action_enum.contains(&serde_json::json!("get_repo")),
"enum should contain get_repo"
);
assert!(
action_enum.contains(&serde_json::json!("list_issues")),
"enum should contain list_issues"
);
assert!(
action.get("const").is_none(),
"const should be removed after merging into enum"
);
// state: has enum → kept
assert!(
props.contains_key("state"),
"state should be kept (has enum)"
);
// owner/repo: not in top-level required, no enum → intentionally dropped
// (variant-level required is omitted; discoverable via tool_info)
assert!(!props.contains_key("owner"), "owner should be dropped");
assert!(!props.contains_key("repo"), "repo should be dropped");
assert_eq!(compacted["additionalProperties"], true);
assert_eq!(compacted["required"], serde_json::json!(["action"]));
}
#[test]
fn test_capabilities_default() {
let caps = Capabilities::default();