Skip to content

Commit 3f6d2ab

Browse files
authored
fix(skill): treat empty url param as absent when installing skills (#1128)
LLMs sometimes pass "" for optional parameters instead of omitting them. Previously, passing url: "" to skill_install would match the explicit-URL branch and attempt to fetch from an empty string, producing an invalid URL error instead of falling back to the catalog lookup. Fix by adding .filter(|s| !s.is_empty()) so an empty url is treated the same as a missing field. A unit test verifies the parameter filtering behaviour directly; the full execute path (catalog lookup + install) requires a real catalog and database and cannot be covered at the unit level.
1 parent f059d50 commit 3f6d2ab

1 file changed

Lines changed: 24 additions & 1 deletion

File tree

src/tools/builtin/skill_tools.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,11 @@ impl Tool for SkillInstallTool {
301301
let content = if let Some(raw) = params.get("content").and_then(|v| v.as_str()) {
302302
// Direct content provided
303303
raw.to_string()
304-
} else if let Some(url) = params.get("url").and_then(|v| v.as_str()) {
304+
} else if let Some(url) = params
305+
.get("url")
306+
.and_then(|v| v.as_str())
307+
.filter(|s| !s.is_empty())
308+
{
305309
// Fetch from explicit URL
306310
fetch_skill_content(url).await?
307311
} else {
@@ -1297,4 +1301,23 @@ mod tests {
12971301
);
12981302
}
12991303
}
1304+
1305+
#[test]
1306+
fn test_empty_url_param_is_treated_as_absent() {
1307+
// LLMs sometimes pass "" for optional parameters instead of omitting them.
1308+
// Before the fix, url: "" would match Some("") and attempt to fetch from an
1309+
// empty URL (failing with an invalid URL error) instead of falling through to
1310+
// the catalog lookup. The full execute path cannot be tested here without a
1311+
// real catalog and database, so this test verifies the parameter filtering
1312+
// behaviour directly.
1313+
let params = serde_json::json!({"name": "my-skill", "url": ""});
1314+
let url = params
1315+
.get("url")
1316+
.and_then(|v| v.as_str())
1317+
.filter(|s| !s.is_empty());
1318+
assert!(
1319+
url.is_none(),
1320+
"empty url string should be treated as absent"
1321+
);
1322+
}
13001323
}

0 commit comments

Comments
 (0)