mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
Compare commits
20
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f470f5db80 | ||
|
|
ca6d9f6ede | ||
|
|
a3c99f2801 | ||
|
|
2b8063a8cf | ||
|
|
3149c91116 | ||
|
|
a71a503870 | ||
|
|
8c2131db48 | ||
|
|
d7024f557f | ||
|
|
99dadcb0ea | ||
|
|
696d6a0bc8 | ||
|
|
ffbc0cd1d4 | ||
|
|
8391415bce | ||
|
|
edca67e8b1 | ||
|
|
7e8c0fbed6 | ||
|
|
6a1301bc5b | ||
|
|
6aae1f8a9e | ||
|
|
7a9396f081 | ||
|
|
6116c885e3 | ||
|
+7 |
a677b20701 | ||
|
|
8c094aec63 |
@@ -78,70 +78,15 @@ jobs:
|
||||
- name: Check lints
|
||||
run: cargo clippy --all --benches --tests --examples ${{ matrix.flags }} -- -D warnings
|
||||
|
||||
no-panics:
|
||||
name: No panics in production code
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v6
|
||||
with:
|
||||
fetch-depth: 0
|
||||
- name: Check for .unwrap(), .expect(), assert!() in production code
|
||||
run: |
|
||||
BASE="${{ github.event.pull_request.base.sha }}"
|
||||
# Get the full diff for .rs files (production only, exclude tests/ directory)
|
||||
DIFF=$(git diff "$BASE"...HEAD -- 'src/**/*.rs' 'crates/**/*.rs' || true)
|
||||
|
||||
if [ -z "$DIFF" ]; then
|
||||
echo "No production Rust changes detected."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Extract added lines, skipping those inside test modules.
|
||||
# Track whether we're inside a test module by watching hunk headers
|
||||
# (lines starting with @@) whose context contains "mod tests" or "#[cfg(test)]".
|
||||
ADDED=$(echo "$DIFF" | awk '
|
||||
/^@@/ {
|
||||
# Hunk context (after the second @@) tells us the function/module scope
|
||||
in_test = (tolower($0) ~ /mod tests/ || $0 ~ /#\[cfg\(test\)\]/ || $0 ~ /#\[test\]/)
|
||||
}
|
||||
/^\+[^+]/ && !in_test { print }
|
||||
' || true)
|
||||
|
||||
if [ -z "$ADDED" ]; then
|
||||
echo "No production Rust changes detected (test-only changes excluded)."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Match panic-inducing patterns, excluding safety suppressions
|
||||
VIOLATIONS=$(echo "$ADDED" \
|
||||
| grep -E '\.(unwrap|expect)\(|[^_]assert(_eq|_ne)?!' \
|
||||
| grep -Ev 'debug_assert|// safety:' \
|
||||
|| true)
|
||||
|
||||
if [ -n "$VIOLATIONS" ]; then
|
||||
echo "::error::Found .unwrap(), .expect(), or assert!() in production code."
|
||||
echo "Production code must use proper error handling instead of panicking."
|
||||
echo "Suppress false positives with an inline '// safety: <reason>' comment."
|
||||
echo ""
|
||||
echo "$VIOLATIONS" | head -20
|
||||
echo ""
|
||||
COUNT=$(echo "$VIOLATIONS" | wc -l | tr -d ' ')
|
||||
echo "Total: $COUNT violation(s)"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "OK: No panic-inducing calls in changed production code."
|
||||
|
||||
# Roll-up job for branch protection
|
||||
code-style:
|
||||
name: Code Style (fmt + clippy + deny)
|
||||
runs-on: ubuntu-latest
|
||||
if: always()
|
||||
needs: [format, clippy, clippy-windows, deny-check, no-panics]
|
||||
needs: [format, clippy, clippy-windows, deny-check]
|
||||
steps:
|
||||
- run: |
|
||||
if [[ "${{ needs.format.result }}" != "success" || "${{ needs.clippy.result }}" != "success" || "${{ needs.deny-check.result }}" != "success" || "${{ needs.no-panics.result }}" != "success" ]]; then
|
||||
if [[ "${{ needs.format.result }}" != "success" || "${{ needs.clippy.result }}" != "success" || "${{ needs.deny-check.result }}" != "success" ]]; then
|
||||
echo "One or more jobs failed"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@@ -52,7 +52,7 @@ jobs:
|
||||
- group: features
|
||||
files: "tests/e2e/scenarios/test_skills.py tests/e2e/scenarios/test_tool_approval.py"
|
||||
- group: extensions
|
||||
files: "tests/e2e/scenarios/test_extensions.py tests/e2e/scenarios/test_extension_oauth.py tests/e2e/scenarios/test_wasm_lifecycle.py tests/e2e/scenarios/test_tool_execution.py tests/e2e/scenarios/test_pairing.py tests/e2e/scenarios/test_oauth_credential_fallback.py tests/e2e/scenarios/test_routine_oauth_credential_injection.py"
|
||||
files: "tests/e2e/scenarios/test_extensions.py tests/e2e/scenarios/test_extension_oauth.py tests/e2e/scenarios/test_wasm_lifecycle.py tests/e2e/scenarios/test_tool_execution.py tests/e2e/scenarios/test_pairing.py"
|
||||
steps:
|
||||
- uses: actions/checkout@v6
|
||||
|
||||
|
||||
@@ -14,10 +14,6 @@
|
||||
|
||||
target/
|
||||
|
||||
# Python
|
||||
__pycache__/
|
||||
*.pyc
|
||||
|
||||
# Benchmark results (local runs, not committed)
|
||||
bench-results/
|
||||
|
||||
|
||||
@@ -132,7 +132,7 @@ fn embed_registry_catalog(root: &Path) {
|
||||
// No registry dir: write empty catalog
|
||||
fs::write(
|
||||
&out_path,
|
||||
r#"{"tools":[],"channels":[],"mcp_servers":[],"bundles":{"bundles":{}}}"#,
|
||||
r#"{"tools":[],"channels":[],"bundles":{"bundles":{}}}"#,
|
||||
)
|
||||
.unwrap();
|
||||
return;
|
||||
@@ -140,7 +140,6 @@ fn embed_registry_catalog(root: &Path) {
|
||||
|
||||
let mut tools = Vec::new();
|
||||
let mut channels = Vec::new();
|
||||
let mut mcp_servers = Vec::new();
|
||||
|
||||
// Collect tool manifests
|
||||
let tools_dir = registry_dir.join("tools");
|
||||
@@ -154,12 +153,6 @@ fn embed_registry_catalog(root: &Path) {
|
||||
collect_json_files(&channels_dir, &mut channels);
|
||||
}
|
||||
|
||||
// Collect MCP server manifests
|
||||
let mcp_servers_dir = registry_dir.join("mcp-servers");
|
||||
if mcp_servers_dir.is_dir() {
|
||||
collect_json_files(&mcp_servers_dir, &mut mcp_servers);
|
||||
}
|
||||
|
||||
// Read bundles
|
||||
let bundles_path = registry_dir.join("_bundles.json");
|
||||
let bundles_raw = if bundles_path.is_file() {
|
||||
@@ -170,10 +163,9 @@ fn embed_registry_catalog(root: &Path) {
|
||||
|
||||
// Build the combined JSON
|
||||
let catalog = format!(
|
||||
r#"{{"tools":[{}],"channels":[{}],"mcp_servers":[{}],"bundles":{}}}"#,
|
||||
r#"{{"tools":[{}],"channels":[{}],"bundles":{}}}"#,
|
||||
tools.join(","),
|
||||
channels.join(","),
|
||||
mcp_servers.join(","),
|
||||
bundles_raw,
|
||||
);
|
||||
|
||||
|
||||
@@ -6,12 +6,6 @@ rust-version = "1.92"
|
||||
description = "Prompt injection defense, input validation, secret leak detection, and safety policy enforcement"
|
||||
authors = ["NEAR AI <[email protected]>"]
|
||||
license = "MIT OR Apache-2.0"
|
||||
homepage = "https://github.com/nearai/ironclaw"
|
||||
repository = "https://github.com/nearai/ironclaw"
|
||||
publish = false
|
||||
|
||||
[package.metadata.dist]
|
||||
dist = false
|
||||
|
||||
[dependencies]
|
||||
aho-corasick = "1"
|
||||
|
||||
@@ -1,9 +0,0 @@
|
||||
{
|
||||
"name": "asana",
|
||||
"display_name": "Asana",
|
||||
"kind": "mcp_server",
|
||||
"description": "Connect to Asana for task management, projects, and team coordination",
|
||||
"keywords": ["tasks", "projects", "management", "team"],
|
||||
"url": "https://mcp.asana.com/v2/mcp",
|
||||
"auth": "dcr"
|
||||
}
|
||||
@@ -1,9 +0,0 @@
|
||||
{
|
||||
"name": "cloudflare",
|
||||
"display_name": "Cloudflare",
|
||||
"kind": "mcp_server",
|
||||
"description": "Connect to Cloudflare for DNS, Workers, KV, and infrastructure management",
|
||||
"keywords": ["cdn", "dns", "workers", "hosting", "infrastructure"],
|
||||
"url": "https://mcp.cloudflare.com/mcp",
|
||||
"auth": "dcr"
|
||||
}
|
||||
@@ -1,9 +0,0 @@
|
||||
{
|
||||
"name": "intercom",
|
||||
"display_name": "Intercom",
|
||||
"kind": "mcp_server",
|
||||
"description": "Connect to Intercom for customer messaging, support, and engagement",
|
||||
"keywords": ["support", "customers", "messaging", "chat", "helpdesk"],
|
||||
"url": "https://mcp.intercom.com/mcp",
|
||||
"auth": "dcr"
|
||||
}
|
||||
@@ -1,9 +0,0 @@
|
||||
{
|
||||
"name": "linear",
|
||||
"display_name": "Linear",
|
||||
"kind": "mcp_server",
|
||||
"description": "Connect to Linear for issue tracking, project management, and team workflows",
|
||||
"keywords": ["issues", "tickets", "project", "tracking", "bugs"],
|
||||
"url": "https://mcp.linear.app/sse",
|
||||
"auth": "dcr"
|
||||
}
|
||||
@@ -1,9 +0,0 @@
|
||||
{
|
||||
"name": "notion",
|
||||
"display_name": "Notion",
|
||||
"kind": "mcp_server",
|
||||
"description": "Connect to Notion for reading and writing pages, databases, and comments",
|
||||
"keywords": ["notes", "wiki", "docs", "pages", "database"],
|
||||
"url": "https://mcp.notion.com/mcp",
|
||||
"auth": "dcr"
|
||||
}
|
||||
@@ -1,9 +0,0 @@
|
||||
{
|
||||
"name": "sentry",
|
||||
"display_name": "Sentry",
|
||||
"kind": "mcp_server",
|
||||
"description": "Connect to Sentry for error tracking, performance monitoring, and debugging",
|
||||
"keywords": ["errors", "monitoring", "debugging", "crashes", "performance"],
|
||||
"url": "https://mcp.sentry.dev/mcp",
|
||||
"auth": "dcr"
|
||||
}
|
||||
@@ -1,9 +0,0 @@
|
||||
{
|
||||
"name": "stripe",
|
||||
"display_name": "Stripe",
|
||||
"kind": "mcp_server",
|
||||
"description": "Connect to Stripe for payment processing, subscriptions, and financial data",
|
||||
"keywords": ["payments", "billing", "subscriptions", "invoices", "finance"],
|
||||
"url": "https://mcp.stripe.com",
|
||||
"auth": "dcr"
|
||||
}
|
||||
@@ -1,6 +1,2 @@
|
||||
[workspace]
|
||||
git_release_enable = false
|
||||
|
||||
[[package]]
|
||||
name = "ironclaw_safety"
|
||||
release = false
|
||||
|
||||
@@ -70,21 +70,19 @@ echo
|
||||
# This is a WARNING, not a hard violation.
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
echo "--- Check 2: .unwrap() / .expect() / assert!() in production code ---"
|
||||
echo "--- Check 2: .unwrap() / .expect() in production code ---"
|
||||
|
||||
# Collect raw matches excluding obvious test-only files and lines.
|
||||
# Also catches assert!(), assert_eq!(), assert_ne!() but NOT debug_assert variants.
|
||||
raw_results=$(grep -rnE '\.(unwrap|expect)\(|[^_]assert(_eq|_ne)?!' src/ \
|
||||
# Collect raw matches excluding obvious test-only files and lines
|
||||
raw_results=$(grep -rn '\.unwrap()\|\.expect(' src/ \
|
||||
--include='*.rs' \
|
||||
| grep -v 'src/main.rs' \
|
||||
| grep -v 'src/testing.rs' \
|
||||
| grep -v 'src/setup/' \
|
||||
| grep -Ev 'debug_assert|// safety:' \
|
||||
|| true)
|
||||
|
||||
if [ -n "$raw_results" ]; then
|
||||
total=$(echo "$raw_results" | wc -l | tr -d ' ')
|
||||
echo "WARNING: ~$total .unwrap()/.expect()/assert!() calls found in src/ (excluding main/testing/setup)."
|
||||
echo "WARNING: ~$total .unwrap()/.expect() calls found in src/ (excluding main/testing/setup)."
|
||||
echo "Many are in test modules; a per-file breakdown helps triage:"
|
||||
echo
|
||||
# Show per-file counts, sorted by count descending, top 15
|
||||
|
||||
@@ -10,7 +10,6 @@
|
||||
# 3. Hardcoded /tmp paths in tests (flaky in parallel runs)
|
||||
# 4. Tool parameters logged without redaction (secret leaks)
|
||||
# 5. Multi-step DB operations without transaction wrapping
|
||||
# 6. .unwrap(), .expect(), assert!() in production code (panics)
|
||||
#
|
||||
# Suppress individual lines with an inline "// safety: <reason>" comment.
|
||||
|
||||
@@ -129,24 +128,6 @@ if [ -n "$DIFF_W_OUTPUT" ]; then
|
||||
fi
|
||||
fi
|
||||
|
||||
# 6. .unwrap(), .expect(), assert!() in production code
|
||||
# Matches added lines containing panic-inducing calls.
|
||||
# Excludes test files, test modules, and debug_assert (compiled out in release).
|
||||
# Suppress with "// safety: <reason>".
|
||||
PROD_DIFF="$DIFF_OUTPUT"
|
||||
# Strip hunks from test-only files (tests/ directory, *_test.rs, test_*.rs)
|
||||
PROD_DIFF=$(echo "$PROD_DIFF" | grep -v '^+++ b/tests/' || true)
|
||||
if echo "$PROD_DIFF" | grep -nE '^\+' \
|
||||
| grep -E '\.(unwrap|expect)\(|[^_]assert(_eq|_ne)?!' \
|
||||
| grep -vE 'debug_assert|// safety:|#\[cfg\(test\)\]|#\[test\]|mod tests' \
|
||||
| head -5 | grep -q .; then
|
||||
warn "PANIC" "Production code must not use .unwrap(), .expect(), or assert!(). Use proper error handling."
|
||||
echo "$PROD_DIFF" | grep -nE '^\+' \
|
||||
| grep -E '\.(unwrap|expect)\(|[^_]assert(_eq|_ne)?!' \
|
||||
| grep -vE 'debug_assert|// safety:|#\[cfg\(test\)\]|#\[test\]|mod tests' \
|
||||
| head -5 | sed 's/^/ /'
|
||||
fi
|
||||
|
||||
if [ "$WARNINGS" -gt 0 ]; then
|
||||
echo ""
|
||||
echo "Found $WARNINGS potential issue(s). Fix them or add '// safety: <reason>' to suppress."
|
||||
|
||||
@@ -152,30 +152,6 @@ pub async fn run_agentic_loop(
|
||||
// Call LLM
|
||||
let output = delegate.call_llm(reasoning, reason_ctx, iteration).await?;
|
||||
|
||||
match &output.result {
|
||||
RespondResult::Text(text) => {
|
||||
tracing::debug!(
|
||||
iteration,
|
||||
len = text.len(),
|
||||
has_suggestions = text.contains("<suggestions>"),
|
||||
response = %text,
|
||||
"LLM text response"
|
||||
);
|
||||
}
|
||||
RespondResult::ToolCalls {
|
||||
tool_calls,
|
||||
content,
|
||||
} => {
|
||||
let names: Vec<&str> = tool_calls.iter().map(|tc| tc.name.as_str()).collect();
|
||||
tracing::debug!(
|
||||
iteration,
|
||||
tools = ?names,
|
||||
has_content = content.is_some(),
|
||||
"LLM tool_calls response"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
match output.result {
|
||||
RespondResult::Text(text) => {
|
||||
// Tool intent nudge: if the LLM says "let me search..." without
|
||||
|
||||
@@ -1051,54 +1051,6 @@ fn strip_internal_tool_call_text(text: &str) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
/// Extract `<suggestions>["...","..."]</suggestions>` from a response string.
|
||||
///
|
||||
/// Returns `(cleaned_text, suggestions)`. The `<suggestions>` block is stripped
|
||||
/// from the text regardless of whether the JSON inside parses successfully.
|
||||
/// Only the **last** `<suggestions>` block is used (closest to end of response).
|
||||
/// Blocks inside markdown code fences are ignored.
|
||||
pub(crate) fn extract_suggestions(text: &str) -> (String, Vec<String>) {
|
||||
use regex::Regex;
|
||||
use std::sync::LazyLock;
|
||||
|
||||
static RE: LazyLock<Regex> = LazyLock::new(|| {
|
||||
Regex::new(r"(?s)<suggestions>\s*(.*?)\s*</suggestions>").expect("valid regex") // safety: constant pattern
|
||||
});
|
||||
|
||||
// Find the position of the last closing code fence to avoid matching inside code blocks
|
||||
let last_code_fence = text.rfind("```").unwrap_or(0);
|
||||
|
||||
// Find all matches, take the last one that's after the last code fence
|
||||
let mut best_match: Option<regex::Match<'_>> = None;
|
||||
let mut best_capture: Option<String> = None;
|
||||
for caps in RE.captures_iter(text) {
|
||||
if let (Some(full), Some(inner)) = (caps.get(0), caps.get(1))
|
||||
&& full.start() >= last_code_fence
|
||||
{
|
||||
best_match = Some(full);
|
||||
best_capture = Some(inner.as_str().to_string());
|
||||
}
|
||||
}
|
||||
|
||||
let Some(full) = best_match else {
|
||||
return (text.to_string(), Vec::new());
|
||||
};
|
||||
|
||||
let cleaned = format!("{}{}", &text[..full.start()], &text[full.end()..]); // safety: regex match boundaries are valid UTF-8
|
||||
let cleaned = cleaned.trim().to_string();
|
||||
|
||||
// Parse the JSON array
|
||||
let suggestions = best_capture
|
||||
.and_then(|json| serde_json::from_str::<Vec<String>>(&json).ok())
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.filter(|s| !s.trim().is_empty() && s.len() <= 80)
|
||||
.take(3)
|
||||
.collect();
|
||||
|
||||
(cleaned, suggestions)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::sync::Arc;
|
||||
@@ -2245,55 +2197,6 @@ mod tests {
|
||||
assert_eq!(result, input);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_suggestions_basic() {
|
||||
let input = "Here is my answer.\n<suggestions>[\"Check logs\", \"Deploy\"]</suggestions>";
|
||||
let (text, suggestions) = super::extract_suggestions(input);
|
||||
assert_eq!(text, "Here is my answer."); // safety: test
|
||||
assert_eq!(suggestions, vec!["Check logs", "Deploy"]); // safety: test
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_suggestions_no_tag() {
|
||||
let input = "Just a plain response.";
|
||||
let (text, suggestions) = super::extract_suggestions(input);
|
||||
assert_eq!(text, "Just a plain response."); // safety: test
|
||||
assert!(suggestions.is_empty()); // safety: test
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_suggestions_malformed_json() {
|
||||
let input = "Answer.\n<suggestions>not json</suggestions>";
|
||||
let (text, suggestions) = super::extract_suggestions(input);
|
||||
assert_eq!(text, "Answer."); // safety: test
|
||||
assert!(suggestions.is_empty()); // safety: test
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_suggestions_inside_code_fence() {
|
||||
let input = "```\n<suggestions>[\"foo\"]</suggestions>\n```";
|
||||
let (text, suggestions) = super::extract_suggestions(input);
|
||||
// The tag is inside a code fence, so it should not be extracted
|
||||
assert_eq!(text, input); // safety: test
|
||||
assert!(suggestions.is_empty()); // safety: test
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_suggestions_after_code_fence() {
|
||||
let input = "```\ncode\n```\nAnswer.\n<suggestions>[\"foo\"]</suggestions>";
|
||||
let (text, suggestions) = super::extract_suggestions(input);
|
||||
assert_eq!(text, "```\ncode\n```\nAnswer."); // safety: test
|
||||
assert_eq!(suggestions, vec!["foo"]); // safety: test
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_suggestions_filters_long() {
|
||||
let long = "x".repeat(81);
|
||||
let input = format!("Answer.\n<suggestions>[\"{}\", \"ok\"]</suggestions>", long);
|
||||
let (_, suggestions) = super::extract_suggestions(&input);
|
||||
assert_eq!(suggestions, vec!["ok"]); // safety: test
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_tool_error_format_includes_tool_name() {
|
||||
// Regression test for issue #487: tool errors sent to the LLM should
|
||||
|
||||
+12
-31
@@ -93,26 +93,19 @@ impl RoutineEngine {
|
||||
let mut cache = Vec::new();
|
||||
for routine in routines {
|
||||
match &routine.trigger {
|
||||
Trigger::Event { pattern, .. } => {
|
||||
// Use RegexBuilder with size limit to prevent ReDoS
|
||||
// from user-supplied patterns (issue #825).
|
||||
match regex::RegexBuilder::new(pattern)
|
||||
.size_limit(64 * 1024) // 64KB compiled size limit
|
||||
.build()
|
||||
{
|
||||
Ok(re) => cache.push(EventMatcher::Message {
|
||||
routine: routine.clone(),
|
||||
regex: re,
|
||||
}),
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
routine = %routine.name,
|
||||
"Invalid or too complex event regex '{}': {}",
|
||||
pattern, e
|
||||
);
|
||||
}
|
||||
Trigger::Event { pattern, .. } => match Regex::new(pattern) {
|
||||
Ok(re) => cache.push(EventMatcher::Message {
|
||||
routine: routine.clone(),
|
||||
regex: re,
|
||||
}),
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
routine = %routine.name,
|
||||
"Invalid event regex '{}': {}",
|
||||
pattern, e
|
||||
);
|
||||
}
|
||||
}
|
||||
},
|
||||
Trigger::SystemEvent { .. } => {
|
||||
cache.push(EventMatcher::System {
|
||||
routine: routine.clone(),
|
||||
@@ -980,18 +973,6 @@ async fn execute_lightweight_with_tools(
|
||||
}
|
||||
};
|
||||
|
||||
// Truncate oversized tool output to prevent unbounded context growth.
|
||||
// Routine tool loops are lightweight and should not accumulate
|
||||
// large payloads across iterations.
|
||||
const MAX_TOOL_OUTPUT_CHARS: usize = 8192;
|
||||
let result_content = if result_content.len() > MAX_TOOL_OUTPUT_CHARS {
|
||||
let truncated = &result_content
|
||||
[..result_content.floor_char_boundary(MAX_TOOL_OUTPUT_CHARS)];
|
||||
format!("{truncated}\n... [output truncated to {MAX_TOOL_OUTPUT_CHARS} chars]")
|
||||
} else {
|
||||
result_content
|
||||
};
|
||||
|
||||
// Add tool result to context
|
||||
messages.push(ChatMessage::tool_result(&tc.id, &tc.name, &result_content));
|
||||
}
|
||||
|
||||
@@ -420,10 +420,6 @@ impl Agent {
|
||||
// Complete, fail, or request approval
|
||||
match result {
|
||||
Ok(AgenticLoopResult::Response(response)) => {
|
||||
// Extract <suggestions> from response text before user sees it
|
||||
let (response, suggestions) =
|
||||
crate::agent::dispatcher::extract_suggestions(&response);
|
||||
|
||||
// Hook: TransformResponse — allow hooks to modify or reject the final response
|
||||
let response = {
|
||||
let event = crate::hooks::HookEvent::ResponseTransform {
|
||||
@@ -477,18 +473,6 @@ impl Agent {
|
||||
)
|
||||
.await;
|
||||
|
||||
// Send suggestions after response (best-effort, rendered by web gateway)
|
||||
if !suggestions.is_empty() {
|
||||
let _ = self
|
||||
.channels
|
||||
.send_status(
|
||||
&message.channel,
|
||||
StatusUpdate::Suggestions { suggestions },
|
||||
&message.metadata,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
Ok(SubmissionResult::response(response))
|
||||
}
|
||||
Ok(AgenticLoopResult::NeedApproval { pending }) => {
|
||||
@@ -1350,8 +1334,6 @@ impl Agent {
|
||||
|
||||
match result {
|
||||
Ok(AgenticLoopResult::Response(response)) => {
|
||||
let (response, suggestions) =
|
||||
crate::agent::dispatcher::extract_suggestions(&response);
|
||||
thread.complete_turn(&response);
|
||||
let (turn_number, tool_calls) = thread
|
||||
.turns
|
||||
@@ -1382,16 +1364,6 @@ impl Agent {
|
||||
&message.metadata,
|
||||
)
|
||||
.await;
|
||||
if !suggestions.is_empty() {
|
||||
let _ = self
|
||||
.channels
|
||||
.send_status(
|
||||
&message.channel,
|
||||
StatusUpdate::Suggestions { suggestions },
|
||||
&message.metadata,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
Ok(SubmissionResult::response(response))
|
||||
}
|
||||
Ok(AgenticLoopResult::NeedApproval {
|
||||
|
||||
+1
-1
@@ -594,7 +594,7 @@ impl AppBuilder {
|
||||
let entries: Vec<_> = catalog
|
||||
.all()
|
||||
.iter()
|
||||
.filter_map(|m| m.to_registry_entry())
|
||||
.map(|m| m.to_registry_entry())
|
||||
.collect();
|
||||
tracing::debug!(
|
||||
count = entries.len(),
|
||||
|
||||
@@ -238,8 +238,6 @@ pub enum StatusUpdate {
|
||||
/// Optional workspace path where the image was saved.
|
||||
path: Option<String>,
|
||||
},
|
||||
/// Suggested follow-up messages for the user.
|
||||
Suggestions { suggestions: Vec<String> },
|
||||
}
|
||||
|
||||
impl StatusUpdate {
|
||||
|
||||
+28
-61
@@ -140,7 +140,7 @@ struct WebhookRequest {
|
||||
content: String,
|
||||
/// Optional thread ID for conversation tracking.
|
||||
thread_id: Option<String>,
|
||||
/// Deprecated: webhook secret in request body. Use X-Hub-Signature-256 header instead.
|
||||
/// Deprecated: webhook secret in request body. Use X-IronClaw-Signature header instead.
|
||||
/// This field is accepted for backward compatibility but will be removed in a future release.
|
||||
secret: Option<String>,
|
||||
/// Whether to wait for a synchronous response.
|
||||
@@ -269,26 +269,23 @@ async fn webhook_handler(
|
||||
let mut fallback_req = None;
|
||||
{
|
||||
let webhook_secret = state.webhook_secret.read().await;
|
||||
let expected_secret = match webhook_secret.as_ref() {
|
||||
Some(secret) => secret.expose_secret(),
|
||||
None => {
|
||||
// No secret configured — reject all requests. This guards against
|
||||
// the secret being cleared at runtime via update_secret(None).
|
||||
// The start() method also prevents startup without a secret, but
|
||||
// this is defense-in-depth for the SIGHUP hot-swap path.
|
||||
return (
|
||||
StatusCode::SERVICE_UNAVAILABLE,
|
||||
Json(WebhookResponse {
|
||||
message_id: Uuid::nil(),
|
||||
status: "error".to_string(),
|
||||
response: Some("Webhook authentication not configured".to_string()),
|
||||
}),
|
||||
)
|
||||
.into_response();
|
||||
}
|
||||
let Some(expected_secret) = webhook_secret.as_ref() else {
|
||||
return (
|
||||
StatusCode::UNAUTHORIZED,
|
||||
Json(WebhookResponse {
|
||||
message_id: Uuid::nil(),
|
||||
status: "error".to_string(),
|
||||
response: Some(
|
||||
"Webhook authentication required: HTTP webhook secret is not configured."
|
||||
.to_string(),
|
||||
),
|
||||
}),
|
||||
)
|
||||
.into_response();
|
||||
};
|
||||
let expected_secret = expected_secret.expose_secret();
|
||||
|
||||
match headers.get("x-hub-signature-256") {
|
||||
match headers.get("x-ironclaw-signature") {
|
||||
Some(raw_signature) => match raw_signature.to_str() {
|
||||
Ok(signature) => {
|
||||
if !verify_hmac_signature(expected_secret, &body, signature) {
|
||||
@@ -325,7 +322,7 @@ async fn webhook_handler(
|
||||
message_id: Uuid::nil(),
|
||||
status: "error".to_string(),
|
||||
response: Some(
|
||||
"Webhook authentication required. Provide X-Hub-Signature-256 header \
|
||||
"Webhook authentication required. Provide X-IronClaw-Signature header \
|
||||
(preferred) or 'secret' field in body (deprecated)."
|
||||
.to_string(),
|
||||
),
|
||||
@@ -341,7 +338,7 @@ async fn webhook_handler(
|
||||
{
|
||||
tracing::warn!(
|
||||
"Webhook authenticated via deprecated 'secret' field in request body. \
|
||||
Migrate to X-Hub-Signature-256 header (HMAC-SHA256). \
|
||||
Migrate to X-IronClaw-Signature header (HMAC-SHA256). \
|
||||
Body secret support will be removed in a future release."
|
||||
);
|
||||
fallback_req = Some(req);
|
||||
@@ -364,7 +361,7 @@ async fn webhook_handler(
|
||||
message_id: Uuid::nil(),
|
||||
status: "error".to_string(),
|
||||
response: Some(
|
||||
"Webhook authentication required. Provide X-Hub-Signature-256 header \
|
||||
"Webhook authentication required. Provide X-IronClaw-Signature header \
|
||||
(preferred) or 'secret' field in body (deprecated)."
|
||||
.to_string(),
|
||||
),
|
||||
@@ -726,7 +723,7 @@ mod tests {
|
||||
.method("POST")
|
||||
.uri("/webhook")
|
||||
.header("content-type", "application/json")
|
||||
.header("x-hub-signature-256", signature)
|
||||
.header("x-ironclaw-signature", signature)
|
||||
.body(Body::from(body_bytes))
|
||||
.unwrap();
|
||||
|
||||
@@ -749,7 +746,7 @@ mod tests {
|
||||
.method("POST")
|
||||
.uri("/webhook")
|
||||
.header("content-type", "application/json")
|
||||
.header("x-hub-signature-256", signature)
|
||||
.header("x-ironclaw-signature", signature)
|
||||
.body(Body::from(body_bytes))
|
||||
.unwrap();
|
||||
|
||||
@@ -770,7 +767,7 @@ mod tests {
|
||||
.method("POST")
|
||||
.uri("/webhook")
|
||||
.header("content-type", "application/json")
|
||||
.header("x-hub-signature-256", "not-a-valid-signature")
|
||||
.header("x-ironclaw-signature", "not-a-valid-signature")
|
||||
.body(Body::from(serde_json::to_vec(&body).unwrap()))
|
||||
.unwrap();
|
||||
|
||||
@@ -919,7 +916,7 @@ mod tests {
|
||||
.method("POST")
|
||||
.uri("/webhook")
|
||||
.header("content-type", "application/json")
|
||||
.header("x-hub-signature-256", signature)
|
||||
.header("x-ironclaw-signature", signature)
|
||||
.body(Body::from(body_bytes))
|
||||
.unwrap();
|
||||
|
||||
@@ -941,7 +938,7 @@ mod tests {
|
||||
.method("POST")
|
||||
.uri("/webhook")
|
||||
.header("content-type", "application/json")
|
||||
.header("x-hub-signature-256", signature)
|
||||
.header("x-ironclaw-signature", signature)
|
||||
.body(Body::from(body))
|
||||
.unwrap();
|
||||
|
||||
@@ -966,7 +963,7 @@ mod tests {
|
||||
.method("POST")
|
||||
.uri("/webhook")
|
||||
.header("content-type", "text/plain")
|
||||
.header("x-hub-signature-256", signature)
|
||||
.header("x-ironclaw-signature", signature)
|
||||
.body(Body::from(body_bytes))
|
||||
.unwrap();
|
||||
|
||||
@@ -991,7 +988,7 @@ mod tests {
|
||||
.body(Body::from(serde_json::to_vec(&body).unwrap()))
|
||||
.unwrap();
|
||||
req.headers_mut().insert(
|
||||
"x-hub-signature-256",
|
||||
"x-ironclaw-signature",
|
||||
HeaderValue::from_bytes(b"\xFF").unwrap(),
|
||||
);
|
||||
|
||||
@@ -1083,12 +1080,12 @@ mod tests {
|
||||
.method("POST")
|
||||
.uri("/webhook")
|
||||
.header("content-type", "application/json")
|
||||
.header("x-hub-signature-256", signature)
|
||||
.header("x-ironclaw-signature", signature)
|
||||
.body(Body::from(body_bytes))
|
||||
.unwrap();
|
||||
|
||||
let resp = app.oneshot(req).await.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE); // safety: test assertion
|
||||
assert_eq!(resp.status(), StatusCode::UNAUTHORIZED);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -1209,34 +1206,4 @@ mod tests {
|
||||
let body = b"test body content";
|
||||
assert!(!verify_hmac_signature(secret, body, "sha256=not-hex!"));
|
||||
}
|
||||
|
||||
/// Regression test for issue #1033: when the webhook secret is cleared at
|
||||
/// runtime via update_secret(None), subsequent requests must be rejected
|
||||
/// instead of being processed without authentication.
|
||||
#[tokio::test]
|
||||
async fn webhook_rejects_when_secret_cleared_at_runtime() {
|
||||
let channel = test_channel(Some("initial-secret"));
|
||||
let _stream = channel.start().await.unwrap();
|
||||
|
||||
// Clear the secret at runtime (simulates a bad SIGHUP config reload)
|
||||
channel.update_secret(None).await;
|
||||
|
||||
let app = channel.routes();
|
||||
let body = serde_json::json!({
|
||||
"content": "hello"
|
||||
});
|
||||
let req = Request::builder()
|
||||
.method("POST")
|
||||
.uri("/webhook")
|
||||
.header("content-type", "application/json")
|
||||
.body(Body::from(serde_json::to_vec(&body).unwrap()))
|
||||
.unwrap();
|
||||
|
||||
let resp = app.oneshot(req).await.unwrap();
|
||||
assert_eq!(
|
||||
resp.status(),
|
||||
StatusCode::SERVICE_UNAVAILABLE,
|
||||
"requests must be rejected when webhook secret is cleared at runtime"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -294,8 +294,6 @@ impl Channel for RelayChannel {
|
||||
match client.connect_stream(&token, stream_timeout_secs).await {
|
||||
Ok((new_stream, new_parser)) => {
|
||||
tracing::info!("Relay SSE stream reconnected");
|
||||
consecutive_failures = 0;
|
||||
backoff_ms = backoff_initial_ms;
|
||||
current_stream = new_stream;
|
||||
// Abort old parser before replacing
|
||||
if let Some(old) = parser_handle.write().await.take() {
|
||||
@@ -314,8 +312,6 @@ impl Channel for RelayChannel {
|
||||
tracing::info!(
|
||||
"Relay SSE stream reconnected with new token"
|
||||
);
|
||||
consecutive_failures = 0;
|
||||
backoff_ms = backoff_initial_ms;
|
||||
current_stream = new_stream;
|
||||
if let Some(old) = parser_handle.write().await.take() {
|
||||
old.abort();
|
||||
|
||||
@@ -607,9 +607,6 @@ impl Channel for ReplChannel {
|
||||
eprintln!("\x1b[36m [image generated]\x1b[0m");
|
||||
}
|
||||
}
|
||||
StatusUpdate::Suggestions { .. } => {
|
||||
// Suggestions are only rendered by the web gateway
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -1664,9 +1664,7 @@ impl WasmChannel {
|
||||
.await;
|
||||
let pairing_store = self.pairing_store.clone();
|
||||
|
||||
let Some(wit_update) = status_to_wit(status, metadata) else {
|
||||
return Ok(());
|
||||
};
|
||||
let wit_update = status_to_wit(status, metadata);
|
||||
|
||||
let result = tokio::time::timeout(timeout, async move {
|
||||
tokio::task::spawn_blocking(move || {
|
||||
@@ -1835,9 +1833,7 @@ impl WasmChannel {
|
||||
.await;
|
||||
let pairing_store = self.pairing_store.clone();
|
||||
let callback_timeout = self.runtime.config().callback_timeout;
|
||||
let Some(wit_update) = status_to_wit(&status, metadata) else {
|
||||
return Ok(());
|
||||
};
|
||||
let wit_update = status_to_wit(&status, metadata);
|
||||
|
||||
let handle = tokio::spawn(async move {
|
||||
let mut interval = tokio::time::interval(Duration::from_secs(4));
|
||||
@@ -2708,13 +2704,10 @@ fn truncate_status_text(input: &str, max_chars: usize) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
fn status_to_wit(
|
||||
status: &StatusUpdate,
|
||||
metadata: &serde_json::Value,
|
||||
) -> Option<wit_channel::StatusUpdate> {
|
||||
fn status_to_wit(status: &StatusUpdate, metadata: &serde_json::Value) -> wit_channel::StatusUpdate {
|
||||
let metadata_json = serde_json::to_string(metadata).unwrap_or_default();
|
||||
|
||||
Some(match status {
|
||||
match status {
|
||||
StatusUpdate::Thinking(msg) => wit_channel::StatusUpdate {
|
||||
status: wit_channel::StatusType::Thinking,
|
||||
message: msg.clone(),
|
||||
@@ -2834,9 +2827,7 @@ fn status_to_wit(
|
||||
},
|
||||
metadata_json,
|
||||
},
|
||||
// Suggestions are web-gateway-only; skip for WASM channels
|
||||
StatusUpdate::Suggestions { .. } => return None,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
/// Clone a WIT StatusUpdate (the generated type doesn't derive Clone).
|
||||
@@ -3565,8 +3556,7 @@ mod tests {
|
||||
let wit = status_to_wit(
|
||||
&crate::channels::StatusUpdate::Thinking("Processing...".into()),
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3584,8 +3574,7 @@ mod tests {
|
||||
let wit = status_to_wit(
|
||||
&crate::channels::StatusUpdate::Status("Done".into()),
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(wit.status, super::wit_channel::StatusType::Done));
|
||||
}
|
||||
@@ -3600,16 +3589,14 @@ mod tests {
|
||||
let wit = status_to_wit(
|
||||
&crate::channels::StatusUpdate::Status("done".into()),
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
assert!(matches!(wit.status, super::wit_channel::StatusType::Done));
|
||||
|
||||
// with whitespace
|
||||
let wit = status_to_wit(
|
||||
&crate::channels::StatusUpdate::Status(" Done ".into()),
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
assert!(matches!(wit.status, super::wit_channel::StatusType::Done));
|
||||
}
|
||||
|
||||
@@ -3621,8 +3608,7 @@ mod tests {
|
||||
let wit = status_to_wit(
|
||||
&crate::channels::StatusUpdate::Status("Interrupted".into()),
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3640,8 +3626,7 @@ mod tests {
|
||||
let wit = status_to_wit(
|
||||
&crate::channels::StatusUpdate::Status("interrupted".into()),
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
super::wit_channel::StatusType::Interrupted
|
||||
@@ -3651,8 +3636,7 @@ mod tests {
|
||||
let wit = status_to_wit(
|
||||
&crate::channels::StatusUpdate::Status(" Interrupted ".into()),
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
super::wit_channel::StatusType::Interrupted
|
||||
@@ -3667,8 +3651,7 @@ mod tests {
|
||||
let wit = status_to_wit(
|
||||
&crate::channels::StatusUpdate::Status("Awaiting approval".into()),
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(wit.status, super::wit_channel::StatusType::Status));
|
||||
assert_eq!(wit.message, "Awaiting approval");
|
||||
@@ -3687,8 +3670,7 @@ mod tests {
|
||||
setup_url: None,
|
||||
},
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3708,8 +3690,7 @@ mod tests {
|
||||
name: "http_request".to_string(),
|
||||
},
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3731,8 +3712,7 @@ mod tests {
|
||||
parameters: None,
|
||||
},
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3754,8 +3734,7 @@ mod tests {
|
||||
parameters: None,
|
||||
},
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3775,8 +3754,7 @@ mod tests {
|
||||
preview: "{".to_string() + "\"temperature\": 22}",
|
||||
},
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3797,8 +3775,7 @@ mod tests {
|
||||
preview: long_preview,
|
||||
},
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3819,8 +3796,7 @@ mod tests {
|
||||
browse_url: "https://example.com/jobs/job-1".to_string(),
|
||||
},
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3842,8 +3818,7 @@ mod tests {
|
||||
message: "Token saved".to_string(),
|
||||
},
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3865,8 +3840,7 @@ mod tests {
|
||||
message: "Invalid token".to_string(),
|
||||
},
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3889,8 +3863,7 @@ mod tests {
|
||||
parameters: serde_json::json!({"url": "https://api.weather.test"}),
|
||||
},
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
@@ -3914,8 +3887,7 @@ mod tests {
|
||||
parameters: serde_json::json!({"url": "https://api.weather.test"}),
|
||||
},
|
||||
&metadata,
|
||||
)
|
||||
.unwrap(); // safety: test
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
wit.status,
|
||||
|
||||
@@ -190,21 +190,12 @@ pub async fn routines_toggle_handler(
|
||||
None => !routine.enabled,
|
||||
};
|
||||
|
||||
// When re-enabling a cron routine, recompute next_fire_at so the cron
|
||||
// ticker can pick it up. Mirrors the CLI behavior (issue #1077).
|
||||
if routine.enabled
|
||||
&& !was_enabled
|
||||
&& let Trigger::Cron {
|
||||
ref schedule,
|
||||
ref timezone,
|
||||
} = routine.trigger
|
||||
&& let Trigger::Cron { schedule, timezone } = &routine.trigger
|
||||
{
|
||||
routine.next_fire_at = next_cron_fire(schedule, timezone.as_deref()).map_err(|e| {
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Failed to compute next fire: {e}"),
|
||||
)
|
||||
})?;
|
||||
routine.next_fire_at = next_cron_fire(schedule, timezone.as_deref())
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
}
|
||||
|
||||
store
|
||||
@@ -212,12 +203,6 @@ pub async fn routines_toggle_handler(
|
||||
.await
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
// Refresh the in-memory event trigger cache so event/system_event
|
||||
// routines reflect the new enabled state immediately (issue #1076).
|
||||
if let Some(engine) = state.routine_engine.read().await.as_ref() {
|
||||
engine.refresh_event_cache().await;
|
||||
}
|
||||
|
||||
Ok(Json(serde_json::json!({
|
||||
"status": if routine.enabled { "enabled" } else { "disabled" },
|
||||
"routine_id": routine_id,
|
||||
@@ -242,12 +227,6 @@ pub async fn routines_delete_handler(
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
if deleted {
|
||||
// Refresh the in-memory event trigger cache so deleted event/system_event
|
||||
// routines stop firing immediately (issue #1076).
|
||||
if let Some(engine) = state.routine_engine.read().await.as_ref() {
|
||||
engine.refresh_event_cache().await;
|
||||
}
|
||||
|
||||
Ok(Json(serde_json::json!({
|
||||
"status": "deleted",
|
||||
"routine_id": routine_id,
|
||||
|
||||
@@ -397,10 +397,6 @@ impl Channel for GatewayChannel {
|
||||
StatusUpdate::ImageGenerated { data_url, path } => SseEvent::ImageGenerated {
|
||||
data_url,
|
||||
path,
|
||||
thread_id: thread_id.clone(),
|
||||
},
|
||||
StatusUpdate::Suggestions { suggestions } => SseEvent::Suggestions {
|
||||
suggestions,
|
||||
thread_id,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -143,7 +143,6 @@ impl SseManager {
|
||||
SseEvent::JobResult { .. } => "job_result",
|
||||
SseEvent::Heartbeat => "heartbeat",
|
||||
SseEvent::ImageGenerated { .. } => "image_generated",
|
||||
SseEvent::Suggestions { .. } => "suggestions",
|
||||
SseEvent::ExtensionStatus { .. } => "extension_status",
|
||||
};
|
||||
Ok(Event::default().event(event_type).data(data))
|
||||
|
||||
@@ -19,7 +19,6 @@ let _loadThreadsTimer = null;
|
||||
const JOB_EVENTS_CAP = 500;
|
||||
const MEMORY_SEARCH_QUERY_MAX_LENGTH = 100;
|
||||
let stagedImages = [];
|
||||
let _ghostSuggestion = '';
|
||||
|
||||
// --- Slash Commands ---
|
||||
|
||||
@@ -287,18 +286,9 @@ function connectSSE() {
|
||||
if (data.thread_id) debouncedLoadThreads();
|
||||
return;
|
||||
}
|
||||
clearSuggestionChips();
|
||||
showActivityThinking(data.message);
|
||||
});
|
||||
|
||||
eventSource.addEventListener('suggestions', (e) => {
|
||||
const data = JSON.parse(e.data);
|
||||
if (!isCurrentThread(data.thread_id)) return;
|
||||
if (data.suggestions && data.suggestions.length > 0) {
|
||||
showSuggestionChips(data.suggestions);
|
||||
}
|
||||
});
|
||||
|
||||
eventSource.addEventListener('tool_started', (e) => {
|
||||
const data = JSON.parse(e.data);
|
||||
if (!isCurrentThread(data.thread_id)) return;
|
||||
@@ -433,59 +423,9 @@ function isCurrentThread(threadId) {
|
||||
return threadId === currentThreadId;
|
||||
}
|
||||
|
||||
// --- Suggestion Chips ---
|
||||
|
||||
function showSuggestionChips(suggestions) {
|
||||
// Clear previous chips/ghost without restoring placeholder (we'll set it below)
|
||||
_ghostSuggestion = '';
|
||||
const container = document.getElementById('suggestion-chips');
|
||||
container.innerHTML = '';
|
||||
const ghost = document.getElementById('ghost-text');
|
||||
ghost.style.display = 'none';
|
||||
const wrapper = document.querySelector('.chat-input-wrapper');
|
||||
if (wrapper) wrapper.classList.remove('has-ghost');
|
||||
|
||||
_ghostSuggestion = suggestions[0] || '';
|
||||
const input = document.getElementById('chat-input');
|
||||
suggestions.forEach(text => {
|
||||
const chip = document.createElement('button');
|
||||
chip.className = 'suggestion-chip';
|
||||
chip.textContent = text;
|
||||
chip.addEventListener('click', () => {
|
||||
input.value = text;
|
||||
clearSuggestionChips();
|
||||
autoResizeTextarea(input);
|
||||
input.focus();
|
||||
sendMessage();
|
||||
});
|
||||
container.appendChild(chip);
|
||||
});
|
||||
container.style.display = 'flex';
|
||||
// Show first suggestion as ghost text in the input so user knows Tab works
|
||||
if (_ghostSuggestion && input.value === '') {
|
||||
ghost.textContent = _ghostSuggestion;
|
||||
ghost.style.display = 'block';
|
||||
input.closest('.chat-input-wrapper').classList.add('has-ghost');
|
||||
}
|
||||
}
|
||||
|
||||
function clearSuggestionChips() {
|
||||
_ghostSuggestion = '';
|
||||
const container = document.getElementById('suggestion-chips');
|
||||
if (container) {
|
||||
container.innerHTML = '';
|
||||
container.style.display = 'none';
|
||||
}
|
||||
const ghost = document.getElementById('ghost-text');
|
||||
if (ghost) ghost.style.display = 'none';
|
||||
const wrapper = document.querySelector('.chat-input-wrapper');
|
||||
if (wrapper) wrapper.classList.remove('has-ghost');
|
||||
}
|
||||
|
||||
// --- Chat ---
|
||||
|
||||
function sendMessage() {
|
||||
clearSuggestionChips();
|
||||
const input = document.getElementById('chat-input');
|
||||
if (!currentThreadId) {
|
||||
console.warn('sendMessage: no thread selected, ignoring');
|
||||
@@ -1394,7 +1334,6 @@ function showAuthCardError(extensionName, message) {
|
||||
}
|
||||
|
||||
function loadHistory(before) {
|
||||
clearSuggestionChips();
|
||||
let historyUrl = '/api/chat/history?limit=50';
|
||||
if (currentThreadId) {
|
||||
historyUrl += '&thread_id=' + encodeURIComponent(currentThreadId);
|
||||
@@ -1690,7 +1629,6 @@ function switchToAssistant() {
|
||||
}
|
||||
|
||||
function switchThread(threadId) {
|
||||
clearSuggestionChips();
|
||||
finalizeActivityGroup();
|
||||
currentThreadId = threadId;
|
||||
unreadThreads.delete(threadId);
|
||||
@@ -1723,15 +1661,6 @@ chatInput.addEventListener('keydown', (e) => {
|
||||
const acEl = document.getElementById('slash-autocomplete');
|
||||
const acVisible = acEl && acEl.style.display !== 'none';
|
||||
|
||||
// Accept first suggestion with Tab (plain Tab only, not Shift+Tab)
|
||||
if (e.key === 'Tab' && !e.shiftKey && !acVisible && _ghostSuggestion && chatInput.value === '') {
|
||||
e.preventDefault();
|
||||
chatInput.value = _ghostSuggestion;
|
||||
clearSuggestionChips();
|
||||
autoResizeTextarea(chatInput);
|
||||
return;
|
||||
}
|
||||
|
||||
if (acVisible) {
|
||||
const items = acEl.querySelectorAll('.slash-ac-item');
|
||||
if (e.key === 'ArrowDown') {
|
||||
@@ -1768,16 +1697,6 @@ chatInput.addEventListener('keydown', (e) => {
|
||||
chatInput.addEventListener('input', () => {
|
||||
autoResizeTextarea(chatInput);
|
||||
filterSlashCommands(chatInput.value);
|
||||
const ghost = document.getElementById('ghost-text');
|
||||
const wrapper = chatInput.closest('.chat-input-wrapper');
|
||||
if (chatInput.value !== '') {
|
||||
ghost.style.display = 'none';
|
||||
wrapper.classList.remove('has-ghost');
|
||||
} else if (_ghostSuggestion) {
|
||||
ghost.textContent = _ghostSuggestion;
|
||||
ghost.style.display = 'block';
|
||||
wrapper.classList.add('has-ghost');
|
||||
}
|
||||
});
|
||||
chatInput.addEventListener('blur', () => {
|
||||
// Small delay so mousedown on autocomplete item fires first
|
||||
|
||||
@@ -155,13 +155,9 @@
|
||||
<div class="chat-container">
|
||||
<div class="chat-messages" id="chat-messages"></div>
|
||||
<div id="slash-autocomplete" class="slash-autocomplete" style="display:none"></div>
|
||||
<div id="suggestion-chips" class="suggestion-chips" style="display:none"></div>
|
||||
<div class="chat-input">
|
||||
<div id="image-preview-strip" class="image-preview-strip"></div>
|
||||
<div class="chat-input-wrapper">
|
||||
<textarea id="chat-input" data-i18n="chat.inputPlaceholder" data-i18n-attr="placeholder" placeholder="Message or / for commands..." rows="1"></textarea>
|
||||
<div id="ghost-text" class="ghost-text"></div>
|
||||
</div>
|
||||
<textarea id="chat-input" data-i18n="chat.inputPlaceholder" data-i18n-attr="placeholder" placeholder="Message or / for commands..." rows="1"></textarea>
|
||||
<input type="file" id="image-file-input" accept="image/*" multiple style="display:none">
|
||||
<button id="attach-btn" class="attach-btn" data-i18n="chat.attachImages" data-i18n-attr="title" title="Attach images"
|
||||
aria-label="Attach images">📎</button>
|
||||
|
||||
@@ -1362,14 +1362,8 @@ body {
|
||||
min-height: 56px;
|
||||
}
|
||||
|
||||
.chat-input-wrapper {
|
||||
position: relative;
|
||||
.chat-input textarea {
|
||||
flex: 1;
|
||||
display: flex;
|
||||
}
|
||||
|
||||
.chat-input-wrapper textarea {
|
||||
width: 100%;
|
||||
padding: 8px 12px;
|
||||
background: var(--bg);
|
||||
border: 1px solid var(--border);
|
||||
@@ -1382,66 +1376,17 @@ body {
|
||||
max-height: 120px;
|
||||
}
|
||||
|
||||
.ghost-text {
|
||||
position: absolute;
|
||||
top: 0;
|
||||
left: 0;
|
||||
right: 0;
|
||||
padding: 8px 12px;
|
||||
font-size: 14px;
|
||||
font-family: inherit;
|
||||
color: var(--text-secondary);
|
||||
opacity: 0.5;
|
||||
pointer-events: none;
|
||||
white-space: pre-wrap;
|
||||
overflow: hidden;
|
||||
display: none;
|
||||
z-index: 1;
|
||||
}
|
||||
|
||||
/* Hide native placeholder when ghost text is visible */
|
||||
.chat-input-wrapper.has-ghost textarea::placeholder {
|
||||
color: transparent;
|
||||
}
|
||||
|
||||
.chat-input-wrapper textarea:focus {
|
||||
.chat-input textarea:focus {
|
||||
outline: none;
|
||||
border-color: var(--accent);
|
||||
box-shadow: 0 0 0 3px rgba(52, 211, 153, 0.1);
|
||||
}
|
||||
|
||||
.chat-input-wrapper textarea:disabled {
|
||||
.chat-input textarea:disabled {
|
||||
opacity: 0.5;
|
||||
cursor: not-allowed;
|
||||
}
|
||||
|
||||
.suggestion-chips {
|
||||
display: none;
|
||||
flex-wrap: wrap;
|
||||
gap: 8px;
|
||||
padding: 8px 16px;
|
||||
border-top: 1px solid var(--border);
|
||||
}
|
||||
|
||||
.suggestion-chip {
|
||||
padding: 6px 14px;
|
||||
background: var(--bg-secondary);
|
||||
border: 1px solid var(--border);
|
||||
border-radius: 16px;
|
||||
color: var(--text-secondary);
|
||||
font-size: 13px;
|
||||
font-family: inherit;
|
||||
cursor: pointer;
|
||||
transition: all 0.15s ease;
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
.suggestion-chip:hover {
|
||||
background: var(--accent);
|
||||
color: #09090b;
|
||||
border-color: var(--accent);
|
||||
}
|
||||
|
||||
.chat-input button {
|
||||
padding: 8px 20px;
|
||||
background: var(--accent);
|
||||
@@ -1471,7 +1416,7 @@ body {
|
||||
}
|
||||
|
||||
/* Keyboard accessibility focus rings */
|
||||
.chat-input-wrapper textarea:focus-visible,
|
||||
.chat-input textarea:focus-visible,
|
||||
.chat-input button:focus-visible,
|
||||
.tab-bar button:focus-visible,
|
||||
.tree-row:focus-visible {
|
||||
@@ -3879,7 +3824,7 @@ mark {
|
||||
min-height: 52px;
|
||||
}
|
||||
|
||||
.chat-input-wrapper textarea {
|
||||
.chat-input textarea {
|
||||
min-height: 36px;
|
||||
max-height: 100px;
|
||||
}
|
||||
|
||||
@@ -242,14 +242,6 @@ pub enum SseEvent {
|
||||
thread_id: Option<String>,
|
||||
},
|
||||
|
||||
/// Suggested follow-up messages for the user.
|
||||
#[serde(rename = "suggestions")]
|
||||
Suggestions {
|
||||
suggestions: Vec<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
thread_id: Option<String>,
|
||||
},
|
||||
|
||||
/// Extension activation status change (WASM channels).
|
||||
#[serde(rename = "extension_status")]
|
||||
ExtensionStatus {
|
||||
@@ -715,7 +707,6 @@ impl WsServerMessage {
|
||||
SseEvent::JobStatus { .. } => "job_status",
|
||||
SseEvent::JobResult { .. } => "job_result",
|
||||
SseEvent::ImageGenerated { .. } => "image_generated",
|
||||
SseEvent::Suggestions { .. } => "suggestions",
|
||||
SseEvent::ExtensionStatus { .. } => "extension_status",
|
||||
};
|
||||
let data = serde_json::to_value(event).unwrap_or(serde_json::Value::Null);
|
||||
|
||||
+6
-18
@@ -127,11 +127,7 @@ fn cmd_list(
|
||||
.unwrap_or("none");
|
||||
println!(
|
||||
"{:<20} {:<8} {:<8} {:<10} {}",
|
||||
m.name,
|
||||
m.kind,
|
||||
m.version.as_deref().unwrap_or("-"),
|
||||
auth,
|
||||
m.description
|
||||
m.name, m.kind, m.version, auth, m.description
|
||||
);
|
||||
} else {
|
||||
println!("{:<20} {:<8} {}", m.name, m.kind, m.description);
|
||||
@@ -177,25 +173,17 @@ fn cmd_info(catalog: &RegistryCatalog, name: &str) -> anyhow::Result<()> {
|
||||
.map_err(|e| anyhow::anyhow!("{}", e))?;
|
||||
|
||||
println!("{} ({})", manifest.display_name, manifest.kind);
|
||||
if let Some(ref version) = manifest.version {
|
||||
println!(" Version: {}", version);
|
||||
}
|
||||
println!(" Version: {}", manifest.version);
|
||||
println!(" {}", manifest.description);
|
||||
|
||||
if !manifest.keywords.is_empty() {
|
||||
println!(" Keywords: {}", manifest.keywords.join(", "));
|
||||
}
|
||||
|
||||
if let Some(ref source) = manifest.source {
|
||||
println!("\nSource:");
|
||||
println!(" Directory: {}", source.dir);
|
||||
println!(" Crate: {}", source.crate_name);
|
||||
println!(" Capabilities: {}", source.capabilities);
|
||||
}
|
||||
|
||||
if let Some(ref url) = manifest.url {
|
||||
println!("\nMCP Server URL: {}", url);
|
||||
}
|
||||
println!("\nSource:");
|
||||
println!(" Directory: {}", manifest.source.dir);
|
||||
println!(" Crate: {}", manifest.source.crate_name);
|
||||
println!(" Capabilities: {}", manifest.source.capabilities);
|
||||
|
||||
if let Some(artifact) = manifest.artifacts.get("wasm32-wasip2") {
|
||||
println!("\nArtifact (wasm32-wasip2):");
|
||||
|
||||
+226
-79
@@ -232,11 +232,198 @@ pub fn builtin_entries() -> Vec<RegistryEntry> {
|
||||
}
|
||||
|
||||
/// Well-known extensions, with an optional relay URL for the channel-relay entry.
|
||||
///
|
||||
/// MCP server entries are loaded from `registry/mcp-servers/*.json` via the catalog
|
||||
/// system. Only runtime-dependent entries (like channel-relay) remain here.
|
||||
pub fn builtin_entries_with_relay(relay_url: Option<String>) -> Vec<RegistryEntry> {
|
||||
let mut entries = vec![];
|
||||
let mut entries = vec![
|
||||
// -- MCP Servers --
|
||||
RegistryEntry {
|
||||
name: "notion".to_string(),
|
||||
display_name: "Notion".to_string(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description: "Connect to Notion for reading and writing pages, databases, and comments"
|
||||
.to_string(),
|
||||
keywords: vec![
|
||||
"notes".into(),
|
||||
"wiki".into(),
|
||||
"docs".into(),
|
||||
"pages".into(),
|
||||
"database".into(),
|
||||
],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://mcp.notion.com/mcp".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
},
|
||||
RegistryEntry {
|
||||
name: "linear".to_string(),
|
||||
display_name: "Linear".to_string(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description:
|
||||
"Connect to Linear for issue tracking, project management, and team workflows"
|
||||
.to_string(),
|
||||
keywords: vec![
|
||||
"issues".into(),
|
||||
"tickets".into(),
|
||||
"project".into(),
|
||||
"tracking".into(),
|
||||
"bugs".into(),
|
||||
],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://mcp.linear.app/sse".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
},
|
||||
RegistryEntry {
|
||||
name: "github".to_string(),
|
||||
display_name: "GitHub".to_string(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description:
|
||||
"Connect to GitHub for repository management, issues, PRs, and code search"
|
||||
.to_string(),
|
||||
keywords: vec![
|
||||
"git".into(),
|
||||
"repos".into(),
|
||||
"code".into(),
|
||||
"pull-request".into(),
|
||||
"issues".into(),
|
||||
],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://api.githubcopilot.com/mcp/".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
},
|
||||
RegistryEntry {
|
||||
name: "slack-mcp".to_string(),
|
||||
display_name: "Slack MCP".to_string(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description:
|
||||
"Connect to Slack via MCP for messaging, channel management, and team communication"
|
||||
.to_string(),
|
||||
keywords: vec![
|
||||
"messaging".into(),
|
||||
"chat".into(),
|
||||
"channels".into(),
|
||||
"team".into(),
|
||||
"communication".into(),
|
||||
],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://mcp.slack.com".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
},
|
||||
RegistryEntry {
|
||||
name: "sentry".to_string(),
|
||||
display_name: "Sentry".to_string(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description:
|
||||
"Connect to Sentry for error tracking, performance monitoring, and debugging"
|
||||
.to_string(),
|
||||
keywords: vec![
|
||||
"errors".into(),
|
||||
"monitoring".into(),
|
||||
"debugging".into(),
|
||||
"crashes".into(),
|
||||
"performance".into(),
|
||||
],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://mcp.sentry.dev/mcp".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
},
|
||||
RegistryEntry {
|
||||
name: "stripe".to_string(),
|
||||
display_name: "Stripe".to_string(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description:
|
||||
"Connect to Stripe for payment processing, subscriptions, and financial data"
|
||||
.to_string(),
|
||||
keywords: vec![
|
||||
"payments".into(),
|
||||
"billing".into(),
|
||||
"subscriptions".into(),
|
||||
"invoices".into(),
|
||||
"finance".into(),
|
||||
],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://mcp.stripe.com".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
},
|
||||
RegistryEntry {
|
||||
name: "cloudflare".to_string(),
|
||||
display_name: "Cloudflare".to_string(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description:
|
||||
"Connect to Cloudflare for DNS, Workers, KV, and infrastructure management"
|
||||
.to_string(),
|
||||
keywords: vec![
|
||||
"cdn".into(),
|
||||
"dns".into(),
|
||||
"workers".into(),
|
||||
"hosting".into(),
|
||||
"infrastructure".into(),
|
||||
],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://mcp.cloudflare.com/mcp".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
},
|
||||
RegistryEntry {
|
||||
name: "asana".to_string(),
|
||||
display_name: "Asana".to_string(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description: "Connect to Asana for task management, projects, and team coordination"
|
||||
.to_string(),
|
||||
keywords: vec![
|
||||
"tasks".into(),
|
||||
"projects".into(),
|
||||
"management".into(),
|
||||
"team".into(),
|
||||
],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://mcp.asana.com/v2/mcp".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
},
|
||||
RegistryEntry {
|
||||
name: "intercom".to_string(),
|
||||
display_name: "Intercom".to_string(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description: "Connect to Intercom for customer messaging, support, and engagement"
|
||||
.to_string(),
|
||||
keywords: vec![
|
||||
"support".into(),
|
||||
"customers".into(),
|
||||
"messaging".into(),
|
||||
"chat".into(),
|
||||
"helpdesk".into(),
|
||||
],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://mcp.intercom.com/mcp".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
},
|
||||
// WASM channels (telegram, slack, discord, whatsapp) come from the embedded
|
||||
// registry catalog (registry/channels/*.json) with WasmDownload URLs pointing
|
||||
// to GitHub release artifacts. See new_with_catalog() for merging.
|
||||
];
|
||||
|
||||
// Conditionally add channel-relay entries when relay URL is configured
|
||||
if let Some(relay_url) = relay_url {
|
||||
@@ -358,21 +545,9 @@ mod tests {
|
||||
assert_eq!(score, 0, "No match should score 0");
|
||||
}
|
||||
|
||||
/// Helper to create a registry with catalog entries (MCP servers come from catalog now).
|
||||
fn registry_with_catalog() -> ExtensionRegistry {
|
||||
let catalog = crate::registry::catalog::RegistryCatalog::load_or_embedded()
|
||||
.expect("catalog should load");
|
||||
let catalog_entries: Vec<RegistryEntry> = catalog
|
||||
.all()
|
||||
.iter()
|
||||
.filter_map(|m| m.to_registry_entry())
|
||||
.collect();
|
||||
ExtensionRegistry::new_with_catalog(catalog_entries)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_search_returns_sorted() {
|
||||
let registry = registry_with_catalog();
|
||||
let registry = ExtensionRegistry::new();
|
||||
let results = registry.search("notion").await;
|
||||
|
||||
assert!(!results.is_empty(), "Should find notion in registry");
|
||||
@@ -381,7 +556,7 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_search_empty_query_returns_all() {
|
||||
let registry = registry_with_catalog();
|
||||
let registry = ExtensionRegistry::new();
|
||||
let results = registry.search("").await;
|
||||
|
||||
assert!(results.len() > 5, "Empty query should return all entries");
|
||||
@@ -389,7 +564,7 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_search_by_keyword() {
|
||||
let registry = registry_with_catalog();
|
||||
let registry = ExtensionRegistry::new();
|
||||
let results = registry.search("issues tickets").await;
|
||||
|
||||
assert!(
|
||||
@@ -403,7 +578,7 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_get_exact_name() {
|
||||
let registry = registry_with_catalog();
|
||||
let registry = ExtensionRegistry::new();
|
||||
|
||||
let entry = registry.get("notion").await;
|
||||
assert!(entry.is_some());
|
||||
@@ -483,30 +658,17 @@ mod tests {
|
||||
auth_hint: AuthHint::CapabilitiesAuth,
|
||||
version: None,
|
||||
},
|
||||
// Two entries with same name but different kinds should coexist
|
||||
// This shares a name with the builtin slack-mcp but has a different kind, so both should appear
|
||||
RegistryEntry {
|
||||
name: "dual-ext".to_string(),
|
||||
display_name: "Dual MCP".to_string(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description: "Dual extension MCP server".to_string(),
|
||||
keywords: vec!["messaging".into()],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://mcp.example.com".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
},
|
||||
RegistryEntry {
|
||||
name: "dual-ext".to_string(),
|
||||
display_name: "Dual WASM".to_string(),
|
||||
name: "slack-mcp".to_string(),
|
||||
display_name: "Slack MCP WASM".to_string(),
|
||||
kind: ExtensionKind::WasmTool,
|
||||
description: "Dual extension WASM tool".to_string(),
|
||||
description: "Slack WASM tool".to_string(),
|
||||
keywords: vec!["messaging".into()],
|
||||
source: ExtensionSource::WasmBuildable {
|
||||
source_dir: "tools-src/dual".to_string(),
|
||||
build_dir: Some("tools-src/dual".to_string()),
|
||||
crate_name: Some("dual-tool".to_string()),
|
||||
source_dir: "tools-src/slack".to_string(),
|
||||
build_dir: Some("tools-src/slack".to_string()),
|
||||
crate_name: Some("slack-tool".to_string()),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::CapabilitiesAuth,
|
||||
@@ -521,56 +683,41 @@ mod tests {
|
||||
assert!(!results.is_empty(), "Should find telegram from catalog");
|
||||
assert_eq!(results[0].entry.name, "telegram");
|
||||
|
||||
// Should have both MCP and WASM entries with the same name
|
||||
let results = registry.search("dual-ext").await;
|
||||
let has_mcp = results
|
||||
// Should have both builtin MCP slack-mcp and catalog WASM slack-mcp
|
||||
let results = registry.search("slack").await;
|
||||
let slack_mcp = results
|
||||
.iter()
|
||||
.any(|r| r.entry.name == "dual-ext" && r.entry.kind == ExtensionKind::McpServer);
|
||||
let has_wasm = results
|
||||
.any(|r| r.entry.name == "slack-mcp" && r.entry.kind == ExtensionKind::McpServer);
|
||||
let slack_wasm = results
|
||||
.iter()
|
||||
.any(|r| r.entry.name == "dual-ext" && r.entry.kind == ExtensionKind::WasmTool);
|
||||
assert!(has_mcp, "Should have MCP dual-ext");
|
||||
assert!(has_wasm, "Should have WASM dual-ext");
|
||||
.any(|r| r.entry.name == "slack-mcp" && r.entry.kind == ExtensionKind::WasmTool);
|
||||
assert!(slack_mcp, "Should have builtin MCP slack-mcp");
|
||||
assert!(slack_wasm, "Should have catalog WASM slack-mcp");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_new_with_catalog_dedup_same_kind() {
|
||||
// When two catalog entries share name AND kind, only the first should be kept
|
||||
let catalog_entries = vec![
|
||||
RegistryEntry {
|
||||
name: "test-ext".to_string(),
|
||||
display_name: "Test First".to_string(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description: "First entry".to_string(),
|
||||
keywords: vec![],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://first.example.com".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
// A catalog entry with same name AND kind as a builtin should be skipped
|
||||
let catalog_entries = vec![RegistryEntry {
|
||||
name: "slack-mcp".to_string(),
|
||||
display_name: "Slack MCP Override".to_string(),
|
||||
kind: ExtensionKind::McpServer, // same kind as builtin slack-mcp
|
||||
description: "Should be skipped".to_string(),
|
||||
keywords: vec![],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://other.slack.com".to_string(),
|
||||
},
|
||||
RegistryEntry {
|
||||
name: "test-ext".to_string(),
|
||||
display_name: "Test Duplicate".to_string(),
|
||||
kind: ExtensionKind::McpServer, // same kind
|
||||
description: "Should be skipped".to_string(),
|
||||
keywords: vec![],
|
||||
source: ExtensionSource::McpUrl {
|
||||
url: "https://second.example.com".to_string(),
|
||||
},
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
},
|
||||
];
|
||||
fallback_source: None,
|
||||
auth_hint: AuthHint::Dcr,
|
||||
version: None,
|
||||
}];
|
||||
|
||||
let registry = ExtensionRegistry::new_with_catalog(catalog_entries);
|
||||
|
||||
let entry = registry.get("test-ext").await;
|
||||
let entry = registry.get("slack-mcp").await;
|
||||
assert!(entry.is_some());
|
||||
// Should be the first entry, not the duplicate
|
||||
assert_eq!(entry.unwrap().display_name, "Test First");
|
||||
// Should still be the builtin, not the override
|
||||
assert_eq!(entry.unwrap().display_name, "Slack MCP");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -270,6 +270,10 @@ impl NearAiChatProvider {
|
||||
reason: format!("Failed to read response body: {}", e),
|
||||
})?;
|
||||
|
||||
if tracing::enabled!(tracing::Level::DEBUG) {
|
||||
tracing::debug!("NEAR AI Chat response status: {}", status);
|
||||
}
|
||||
|
||||
// Log response body only at TRACE level to avoid exposing sensitive content
|
||||
// (user-generated data, tool outputs, leaked secrets) in DEBUG logs
|
||||
if tracing::enabled!(tracing::Level::TRACE) {
|
||||
|
||||
@@ -902,8 +902,7 @@ Example:
|
||||
## Guidelines
|
||||
- Be concise and direct
|
||||
- Use markdown formatting where helpful
|
||||
- For code, use appropriate code blocks with language tags
|
||||
- ALWAYS end your response with a <suggestions> tag containing a JSON array of 1-3 short follow-up commands. Each suggestion must read as something the USER would type to instruct YOU. Write them in the user's voice as direct commands, not as requests FROM you TO the user. Do NOT repeat or rephrase content already in your response. Example: <suggestions>["Suggest dinner spots in my area", "Find a quick recipe for pasta"]</suggestions> Keep each under 80 characters.{}
|
||||
- For code, use appropriate code blocks with language tags{}
|
||||
|
||||
## Safety
|
||||
- You have no independent goals. Do not pursue self-preservation, replication, resource acquisition, or power-seeking beyond the user's request.
|
||||
|
||||
+31
-86
@@ -192,12 +192,6 @@ impl RegistryCatalog {
|
||||
Self::load_manifests_from_dir(&channels_dir, "channels", &mut manifests)?;
|
||||
}
|
||||
|
||||
// Load MCP servers
|
||||
let mcp_servers_dir = registry_dir.join("mcp-servers");
|
||||
if mcp_servers_dir.is_dir() {
|
||||
Self::load_manifests_from_dir(&mcp_servers_dir, "mcp-servers", &mut manifests)?;
|
||||
}
|
||||
|
||||
// Load bundles
|
||||
let bundles_path = registry_dir.join("_bundles.json");
|
||||
let bundles = if bundles_path.is_file() {
|
||||
@@ -286,9 +280,8 @@ impl RegistryCatalog {
|
||||
/// Get a manifest by name. Tries exact key match first ("tools/github"),
|
||||
/// then searches by bare name ("github").
|
||||
///
|
||||
/// If a bare name matches more than one prefix, returns `None`.
|
||||
/// Use a qualified key ("tools/github", "channels/telegram", or
|
||||
/// "mcp-servers/notion") to disambiguate.
|
||||
/// If a bare name matches both a tool and a channel, returns `None`.
|
||||
/// Use a qualified key ("tools/github" or "channels/telegram") to disambiguate.
|
||||
pub fn get(&self, name: &str) -> Option<&ExtensionManifest> {
|
||||
// Try exact key first
|
||||
if let Some(m) = self.manifests.get(name) {
|
||||
@@ -296,15 +289,14 @@ impl RegistryCatalog {
|
||||
}
|
||||
|
||||
// Try with kind prefix, detecting collisions
|
||||
let candidates: Vec<_> = ["tools", "channels", "mcp-servers"]
|
||||
.iter()
|
||||
.filter_map(|prefix| self.manifests.get(&format!("{}/{}", prefix, name)))
|
||||
.collect();
|
||||
let tool = self.manifests.get(&format!("tools/{}", name));
|
||||
let channel = self.manifests.get(&format!("channels/{}", name));
|
||||
|
||||
if candidates.len() == 1 {
|
||||
Some(candidates[0])
|
||||
} else {
|
||||
None // ambiguous or not found
|
||||
match (tool, channel) {
|
||||
(Some(_), Some(_)) => None, // ambiguous
|
||||
(Some(m), None) => Some(m),
|
||||
(None, Some(m)) => Some(m),
|
||||
(None, None) => None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -316,63 +308,37 @@ impl RegistryCatalog {
|
||||
return Ok(m);
|
||||
}
|
||||
|
||||
let prefixes: &[(&str, &str)] = &[
|
||||
("tools", "tool"),
|
||||
("channels", "channel"),
|
||||
("mcp-servers", "mcp_server"),
|
||||
];
|
||||
let has_tool = self.manifests.contains_key(&format!("tools/{}", name));
|
||||
let has_channel = self.manifests.contains_key(&format!("channels/{}", name));
|
||||
|
||||
let matches: Vec<_> = prefixes
|
||||
.iter()
|
||||
.filter(|(prefix, _)| self.manifests.contains_key(&format!("{}/{}", prefix, name)))
|
||||
.collect();
|
||||
|
||||
match matches.len() {
|
||||
0 => Err(RegistryError::ExtensionNotFound(name.to_string())),
|
||||
1 => {
|
||||
let (prefix, _) = matches[0];
|
||||
let key = format!("{}/{}", prefix, name);
|
||||
self.manifests
|
||||
.get(&key)
|
||||
.ok_or_else(|| RegistryError::ExtensionNotFound(name.to_string()))
|
||||
}
|
||||
_ => {
|
||||
let (prefix_a, kind_a) = matches[0];
|
||||
let (prefix_b, kind_b) = matches[1];
|
||||
Err(RegistryError::AmbiguousName {
|
||||
name: name.to_string(),
|
||||
kind_a,
|
||||
prefix_a,
|
||||
kind_b,
|
||||
prefix_b,
|
||||
})
|
||||
}
|
||||
match (has_tool, has_channel) {
|
||||
(true, true) => Err(RegistryError::AmbiguousName {
|
||||
name: name.to_string(),
|
||||
kind_a: "tool",
|
||||
prefix_a: "tools",
|
||||
kind_b: "channel",
|
||||
prefix_b: "channels",
|
||||
}),
|
||||
(true, false) => Ok(self.manifests.get(&format!("tools/{}", name)).unwrap()),
|
||||
(false, true) => Ok(self.manifests.get(&format!("channels/{}", name)).unwrap()),
|
||||
(false, false) => Err(RegistryError::ExtensionNotFound(name.to_string())),
|
||||
}
|
||||
}
|
||||
|
||||
/// Get the full key ("tools/github", "channels/telegram", or
|
||||
/// "mcp-servers/notion") for a manifest.
|
||||
/// Get the full key ("tools/github" or "channels/telegram") for a manifest.
|
||||
pub fn key_for(&self, name: &str) -> Option<String> {
|
||||
if self.manifests.contains_key(name) {
|
||||
return Some(name.to_string());
|
||||
}
|
||||
|
||||
let matches: Vec<String> = ["tools", "channels", "mcp-servers"]
|
||||
.iter()
|
||||
.filter_map(|prefix| {
|
||||
let key = format!("{}/{}", prefix, name);
|
||||
if self.manifests.contains_key(&key) {
|
||||
Some(key)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
let has_tool = self.manifests.contains_key(&format!("tools/{}", name));
|
||||
let has_channel = self.manifests.contains_key(&format!("channels/{}", name));
|
||||
|
||||
if matches.len() == 1 {
|
||||
matches.into_iter().next()
|
||||
} else {
|
||||
None // ambiguous or not found
|
||||
match (has_tool, has_channel) {
|
||||
(true, true) => None, // ambiguous
|
||||
(true, false) => Some(format!("tools/{}", name)),
|
||||
(false, true) => Some(format!("channels/{}", name)),
|
||||
(false, false) => None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -510,10 +476,8 @@ mod tests {
|
||||
fn create_test_registry(dir: &Path) {
|
||||
let tools_dir = dir.join("tools");
|
||||
let channels_dir = dir.join("channels");
|
||||
let mcp_dir = dir.join("mcp-servers");
|
||||
fs::create_dir_all(&tools_dir).unwrap();
|
||||
fs::create_dir_all(&channels_dir).unwrap();
|
||||
fs::create_dir_all(&mcp_dir).unwrap();
|
||||
|
||||
fs::write(
|
||||
tools_dir.join("slack.json"),
|
||||
@@ -576,20 +540,6 @@ mod tests {
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
fs::write(
|
||||
mcp_dir.join("notion.json"),
|
||||
r#"{
|
||||
"name": "notion",
|
||||
"display_name": "Notion",
|
||||
"kind": "mcp_server",
|
||||
"description": "Connect to Notion for pages and databases",
|
||||
"keywords": ["notes", "wiki"],
|
||||
"url": "https://mcp.notion.com/mcp",
|
||||
"auth": "dcr"
|
||||
}"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
fs::write(
|
||||
dir.join("_bundles.json"),
|
||||
r#"{
|
||||
@@ -615,7 +565,7 @@ mod tests {
|
||||
create_test_registry(tmp.path());
|
||||
|
||||
let catalog = RegistryCatalog::load(tmp.path()).unwrap();
|
||||
assert_eq!(catalog.all().len(), 4);
|
||||
assert_eq!(catalog.all().len(), 3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -629,9 +579,6 @@ mod tests {
|
||||
|
||||
let channels = catalog.list(Some(ManifestKind::Channel), None);
|
||||
assert_eq!(channels.len(), 1);
|
||||
|
||||
let mcp_servers = catalog.list(Some(ManifestKind::McpServer), None);
|
||||
assert_eq!(mcp_servers.len(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -656,12 +603,10 @@ mod tests {
|
||||
|
||||
// Full key
|
||||
assert!(catalog.get("tools/slack").is_some());
|
||||
assert!(catalog.get("mcp-servers/notion").is_some());
|
||||
|
||||
// Bare name
|
||||
assert!(catalog.get("slack").is_some());
|
||||
assert!(catalog.get("telegram").is_some());
|
||||
assert!(catalog.get("notion").is_some());
|
||||
|
||||
// Missing
|
||||
assert!(catalog.get("nonexistent").is_none());
|
||||
|
||||
@@ -20,8 +20,6 @@ struct EmbeddedCatalogRaw {
|
||||
#[serde(default)]
|
||||
channels: Vec<ExtensionManifest>,
|
||||
#[serde(default)]
|
||||
mcp_servers: Vec<ExtensionManifest>,
|
||||
#[serde(default)]
|
||||
bundles: BundlesFile,
|
||||
}
|
||||
|
||||
@@ -54,10 +52,6 @@ fn parsed_catalog() -> &'static ParsedCatalog {
|
||||
let key = format!("channels/{}", m.name);
|
||||
manifests.insert(key, m);
|
||||
}
|
||||
for m in raw.mcp_servers {
|
||||
let key = format!("mcp-servers/{}", m.name);
|
||||
manifests.insert(key, m);
|
||||
}
|
||||
|
||||
ParsedCatalog {
|
||||
manifests,
|
||||
|
||||
+18
-76
@@ -7,7 +7,7 @@ use tokio::fs;
|
||||
|
||||
use crate::bootstrap::ironclaw_base_dir;
|
||||
use crate::registry::catalog::RegistryError;
|
||||
use crate::registry::manifest::{BundleDefinition, ExtensionManifest, ManifestKind, SourceSpec};
|
||||
use crate::registry::manifest::{BundleDefinition, ExtensionManifest, ManifestKind};
|
||||
|
||||
// GitHub-only by design. New trusted hosts (e.g. a NEAR AI CDN) must be
|
||||
// explicitly added here; unknown hosts fall back to source build with a
|
||||
@@ -98,29 +98,12 @@ fn validate_manifest_install_inputs(manifest: &ExtensionManifest) -> Result<(),
|
||||
});
|
||||
}
|
||||
|
||||
// MCP servers are not installed via this path
|
||||
if manifest.kind == ManifestKind::McpServer {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let source = match &manifest.source {
|
||||
Some(s) => s,
|
||||
None => {
|
||||
return Err(RegistryError::InvalidManifest {
|
||||
name: manifest.name.clone(),
|
||||
field: "source",
|
||||
reason: "WASM extensions must have a source spec".to_string(),
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
let expected_prefix = match manifest.kind {
|
||||
ManifestKind::Tool => "tools-src/",
|
||||
ManifestKind::Channel => "channels-src/",
|
||||
ManifestKind::McpServer => unreachable!(),
|
||||
};
|
||||
|
||||
if !source.dir.starts_with(expected_prefix) {
|
||||
if !manifest.source.dir.starts_with(expected_prefix) {
|
||||
return Err(RegistryError::InvalidManifest {
|
||||
name: manifest.name.clone(),
|
||||
field: "source.dir",
|
||||
@@ -128,7 +111,7 @@ fn validate_manifest_install_inputs(manifest: &ExtensionManifest) -> Result<(),
|
||||
});
|
||||
}
|
||||
|
||||
let source_path = Path::new(&source.dir);
|
||||
let source_path = Path::new(&manifest.source.dir);
|
||||
let has_unsafe_component = source_path.components().any(|component| {
|
||||
matches!(
|
||||
component,
|
||||
@@ -144,9 +127,9 @@ fn validate_manifest_install_inputs(manifest: &ExtensionManifest) -> Result<(),
|
||||
});
|
||||
}
|
||||
|
||||
let has_path_separator = source.capabilities.contains('/')
|
||||
|| source.capabilities.contains('\\')
|
||||
|| source.capabilities.contains("..");
|
||||
let has_path_separator = manifest.source.capabilities.contains('/')
|
||||
|| manifest.source.capabilities.contains('\\')
|
||||
|| manifest.source.capabilities.contains("..");
|
||||
|
||||
if has_path_separator {
|
||||
return Err(RegistryError::InvalidManifest {
|
||||
@@ -159,18 +142,6 @@ fn validate_manifest_install_inputs(manifest: &ExtensionManifest) -> Result<(),
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Extract the source spec from a manifest, returning an error if absent.
|
||||
fn require_source(manifest: &ExtensionManifest) -> Result<&SourceSpec, RegistryError> {
|
||||
manifest
|
||||
.source
|
||||
.as_ref()
|
||||
.ok_or_else(|| RegistryError::InvalidManifest {
|
||||
name: manifest.name.clone(),
|
||||
field: "source",
|
||||
reason: "WASM extensions must have a source spec".to_string(),
|
||||
})
|
||||
}
|
||||
|
||||
fn download_failure_reason(error: &reqwest::Error) -> String {
|
||||
if error.is_timeout() {
|
||||
"request timed out".to_string()
|
||||
@@ -235,17 +206,7 @@ impl RegistryInstaller {
|
||||
) -> Result<InstallOutcome, RegistryError> {
|
||||
validate_manifest_install_inputs(manifest)?;
|
||||
|
||||
if manifest.kind == ManifestKind::McpServer {
|
||||
return Err(RegistryError::InvalidManifest {
|
||||
name: manifest.name.clone(),
|
||||
field: "kind",
|
||||
reason: "MCP servers cannot be installed from source".to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
let source = require_source(manifest)?;
|
||||
|
||||
let source_dir = self.repo_root.join(&source.dir);
|
||||
let source_dir = self.repo_root.join(&manifest.source.dir);
|
||||
if !source_dir.exists() {
|
||||
return Err(RegistryError::ManifestRead {
|
||||
path: source_dir.clone(),
|
||||
@@ -256,7 +217,6 @@ impl RegistryInstaller {
|
||||
let target_dir = match manifest.kind {
|
||||
ManifestKind::Tool => &self.tools_dir,
|
||||
ManifestKind::Channel => &self.channels_dir,
|
||||
ManifestKind::McpServer => unreachable!(),
|
||||
};
|
||||
|
||||
fs::create_dir_all(target_dir)
|
||||
@@ -282,7 +242,7 @@ impl RegistryInstaller {
|
||||
manifest.display_name,
|
||||
source_dir.display()
|
||||
);
|
||||
let crate_name = &source.crate_name;
|
||||
let crate_name = &manifest.source.crate_name;
|
||||
let wasm_path =
|
||||
crate::registry::artifacts::build_wasm_component(&source_dir, crate_name, true)
|
||||
.await
|
||||
@@ -298,7 +258,7 @@ impl RegistryInstaller {
|
||||
.map_err(RegistryError::Io)?;
|
||||
|
||||
// Copy capabilities file
|
||||
let caps_source = source_dir.join(&source.capabilities);
|
||||
let caps_source = source_dir.join(&manifest.source.capabilities);
|
||||
let target_caps = target_dir.join(format!("{}.capabilities.json", manifest.name));
|
||||
let has_capabilities = if caps_source.exists() {
|
||||
fs::copy(&caps_source, &target_caps)
|
||||
@@ -336,16 +296,6 @@ impl RegistryInstaller {
|
||||
// catch it first.
|
||||
validate_manifest_install_inputs(manifest)?;
|
||||
|
||||
if manifest.kind == ManifestKind::McpServer {
|
||||
return Err(RegistryError::InvalidManifest {
|
||||
name: manifest.name.clone(),
|
||||
field: "kind",
|
||||
reason: "MCP servers cannot be installed via the WASM installer".to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
let source = require_source(manifest)?;
|
||||
|
||||
let has_artifact = manifest
|
||||
.artifacts
|
||||
.get("wasm32-wasip2")
|
||||
@@ -356,7 +306,7 @@ impl RegistryInstaller {
|
||||
return self.install_from_source(manifest, force).await;
|
||||
}
|
||||
|
||||
let source_dir = self.repo_root.join(&source.dir);
|
||||
let source_dir = self.repo_root.join(&manifest.source.dir);
|
||||
|
||||
match self.install_from_artifact(manifest, force).await {
|
||||
Ok(outcome) => Ok(outcome),
|
||||
@@ -441,13 +391,6 @@ impl RegistryInstaller {
|
||||
let target_dir = match manifest.kind {
|
||||
ManifestKind::Tool => &self.tools_dir,
|
||||
ManifestKind::Channel => &self.channels_dir,
|
||||
ManifestKind::McpServer => {
|
||||
return Err(RegistryError::InvalidManifest {
|
||||
name: manifest.name.clone(),
|
||||
field: "kind",
|
||||
reason: "MCP servers cannot be installed as artifacts".to_string(),
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
fs::create_dir_all(target_dir)
|
||||
@@ -515,9 +458,12 @@ impl RegistryInstaller {
|
||||
false
|
||||
}
|
||||
}
|
||||
} else if let Some(ref source) = manifest.source {
|
||||
} else {
|
||||
// Legacy fallback: try source tree
|
||||
let caps_source = self.repo_root.join(&source.dir).join(&source.capabilities);
|
||||
let caps_source = self
|
||||
.repo_root
|
||||
.join(&manifest.source.dir)
|
||||
.join(&manifest.source.capabilities);
|
||||
if caps_source.exists() {
|
||||
fs::copy(&caps_source, &target_caps)
|
||||
.await
|
||||
@@ -526,8 +472,6 @@ impl RegistryInstaller {
|
||||
} else {
|
||||
false
|
||||
}
|
||||
} else {
|
||||
false
|
||||
}
|
||||
};
|
||||
|
||||
@@ -831,19 +775,17 @@ mod tests {
|
||||
name: name.to_string(),
|
||||
display_name: name.to_string(),
|
||||
kind,
|
||||
version: Some("0.1.0".to_string()),
|
||||
version: "0.1.0".to_string(),
|
||||
description: "test manifest".to_string(),
|
||||
keywords: Vec::new(),
|
||||
source: Some(SourceSpec {
|
||||
source: SourceSpec {
|
||||
dir: source_dir.to_string(),
|
||||
capabilities: format!("{}.capabilities.json", name),
|
||||
crate_name: name.to_string(),
|
||||
}),
|
||||
},
|
||||
artifacts,
|
||||
auth_summary: None,
|
||||
tags: Vec::new(),
|
||||
url: None,
|
||||
auth: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+21
-192
@@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::extensions::{AuthHint, ExtensionKind, ExtensionSource, RegistryEntry};
|
||||
|
||||
/// A single extension manifest loaded from `registry/{tools,channels,mcp-servers}/<name>.json`.
|
||||
/// A single extension manifest loaded from `registry/{tools,channels}/<name>.json`.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct ExtensionManifest {
|
||||
/// Unique identifier (matches crate name stem, e.g. "slack").
|
||||
@@ -16,12 +16,11 @@ pub struct ExtensionManifest {
|
||||
/// Human-readable name (e.g. "Slack").
|
||||
pub display_name: String,
|
||||
|
||||
/// Whether this is a tool, channel, or MCP server.
|
||||
/// Whether this is a tool or channel.
|
||||
pub kind: ManifestKind,
|
||||
|
||||
/// Semver version from Cargo.toml. Optional for MCP server manifests.
|
||||
#[serde(default)]
|
||||
pub version: Option<String>,
|
||||
/// Semver version from Cargo.toml.
|
||||
pub version: String,
|
||||
|
||||
/// One-line description.
|
||||
pub description: String,
|
||||
@@ -30,9 +29,8 @@ pub struct ExtensionManifest {
|
||||
#[serde(default)]
|
||||
pub keywords: Vec<String>,
|
||||
|
||||
/// Source code location and build info. Absent for MCP server manifests.
|
||||
#[serde(default)]
|
||||
pub source: Option<SourceSpec>,
|
||||
/// Source code location and build info.
|
||||
pub source: SourceSpec,
|
||||
|
||||
/// Pre-built binary artifacts keyed by target triple.
|
||||
#[serde(default)]
|
||||
@@ -45,15 +43,6 @@ pub struct ExtensionManifest {
|
||||
/// Tags for filtering (e.g. "default", "messaging", "google").
|
||||
#[serde(default)]
|
||||
pub tags: Vec<String>,
|
||||
|
||||
/// MCP server URL. Only present for `McpServer` manifests.
|
||||
#[serde(default)]
|
||||
pub url: Option<String>,
|
||||
|
||||
/// MCP auth method: "dcr", "oauth_pre_configured:<setup_url>", or "none".
|
||||
/// Only present for `McpServer` manifests.
|
||||
#[serde(default)]
|
||||
pub auth: Option<String>,
|
||||
}
|
||||
|
||||
/// Extension kind as declared in manifests.
|
||||
@@ -62,7 +51,6 @@ pub struct ExtensionManifest {
|
||||
pub enum ManifestKind {
|
||||
Tool,
|
||||
Channel,
|
||||
McpServer,
|
||||
}
|
||||
|
||||
impl From<ManifestKind> for ExtensionKind {
|
||||
@@ -70,7 +58,6 @@ impl From<ManifestKind> for ExtensionKind {
|
||||
match kind {
|
||||
ManifestKind::Tool => ExtensionKind::WasmTool,
|
||||
ManifestKind::Channel => ExtensionKind::WasmChannel,
|
||||
ManifestKind::McpServer => ExtensionKind::McpServer,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -80,7 +67,6 @@ impl std::fmt::Display for ManifestKind {
|
||||
match self {
|
||||
ManifestKind::Tool => write!(f, "tool"),
|
||||
ManifestKind::Channel => write!(f, "channel"),
|
||||
ManifestKind::McpServer => write!(f, "mcp_server"),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -167,64 +153,12 @@ pub struct BundlesFile {
|
||||
impl ExtensionManifest {
|
||||
/// Convert this manifest into a [`RegistryEntry`] for use with the in-chat
|
||||
/// extension discovery system.
|
||||
///
|
||||
/// Returns `None` for MCP server manifests missing a `url` field.
|
||||
pub fn to_registry_entry(&self) -> Option<RegistryEntry> {
|
||||
if self.kind == ManifestKind::McpServer {
|
||||
return self.to_mcp_registry_entry();
|
||||
}
|
||||
|
||||
Some(self.to_wasm_registry_entry())
|
||||
}
|
||||
|
||||
/// Build a [`RegistryEntry`] for an MCP server manifest.
|
||||
fn to_mcp_registry_entry(&self) -> Option<RegistryEntry> {
|
||||
let url = match &self.url {
|
||||
Some(u) => u.clone(),
|
||||
None => {
|
||||
tracing::warn!(
|
||||
"MCP server manifest '{}' is missing 'url' field, skipping",
|
||||
self.name
|
||||
);
|
||||
return None;
|
||||
}
|
||||
pub fn to_registry_entry(&self) -> RegistryEntry {
|
||||
let buildable = ExtensionSource::WasmBuildable {
|
||||
source_dir: self.source.dir.clone(),
|
||||
build_dir: Some(self.source.dir.clone()),
|
||||
crate_name: Some(self.source.crate_name.clone()),
|
||||
};
|
||||
let auth_hint = match self.auth.as_deref() {
|
||||
Some("dcr") | None => AuthHint::Dcr,
|
||||
Some("none") => AuthHint::None,
|
||||
Some(other) if other.starts_with("oauth_pre_configured:") => {
|
||||
AuthHint::OAuthPreConfigured {
|
||||
setup_url: other
|
||||
.strip_prefix("oauth_pre_configured:")
|
||||
.unwrap_or("")
|
||||
.to_string(),
|
||||
}
|
||||
}
|
||||
_ => AuthHint::Dcr,
|
||||
};
|
||||
|
||||
Some(RegistryEntry {
|
||||
name: self.name.clone(),
|
||||
display_name: self.display_name.clone(),
|
||||
kind: ExtensionKind::McpServer,
|
||||
description: self.description.clone(),
|
||||
keywords: self.keywords.clone(),
|
||||
source: ExtensionSource::McpUrl { url },
|
||||
fallback_source: None,
|
||||
auth_hint,
|
||||
version: self.version.clone(),
|
||||
})
|
||||
}
|
||||
|
||||
/// Build a [`RegistryEntry`] for a WASM tool or channel manifest.
|
||||
fn to_wasm_registry_entry(&self) -> RegistryEntry {
|
||||
let source_spec = self.source.as_ref();
|
||||
|
||||
let buildable = source_spec.map(|s| ExtensionSource::WasmBuildable {
|
||||
source_dir: s.dir.clone(),
|
||||
build_dir: Some(s.dir.clone()),
|
||||
crate_name: Some(s.crate_name.clone()),
|
||||
});
|
||||
|
||||
// Prefer pre-built artifact download when a URL is available,
|
||||
// with build-from-source as fallback in case the download fails (e.g., 404).
|
||||
@@ -236,32 +170,13 @@ impl ExtensionManifest {
|
||||
wasm_url: url.clone(),
|
||||
capabilities_url: artifact.capabilities_url.clone(),
|
||||
},
|
||||
buildable.map(Box::new),
|
||||
Some(Box::new(buildable)),
|
||||
)
|
||||
} else if let Some(b) = buildable {
|
||||
(b, None)
|
||||
} else {
|
||||
// No source spec and no download URL — use a placeholder
|
||||
(
|
||||
ExtensionSource::WasmBuildable {
|
||||
source_dir: String::new(),
|
||||
build_dir: None,
|
||||
crate_name: None,
|
||||
},
|
||||
None,
|
||||
)
|
||||
(buildable, None)
|
||||
}
|
||||
} else if let Some(b) = buildable {
|
||||
(b, None)
|
||||
} else {
|
||||
(
|
||||
ExtensionSource::WasmBuildable {
|
||||
source_dir: String::new(),
|
||||
build_dir: None,
|
||||
crate_name: None,
|
||||
},
|
||||
None,
|
||||
)
|
||||
(buildable, None)
|
||||
};
|
||||
|
||||
let auth_hint = match self.auth_summary.as_ref().and_then(|a| a.method.as_deref()) {
|
||||
@@ -280,7 +195,7 @@ impl ExtensionManifest {
|
||||
source,
|
||||
fallback_source,
|
||||
auth_hint,
|
||||
version: self.version.clone(),
|
||||
version: Some(self.version.clone()),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -319,10 +234,10 @@ mod tests {
|
||||
let manifest: ExtensionManifest = serde_json::from_str(json).expect("parse manifest");
|
||||
assert_eq!(manifest.name, "slack");
|
||||
assert_eq!(manifest.kind, ManifestKind::Tool);
|
||||
assert_eq!(manifest.version.as_deref(), Some("0.1.0"));
|
||||
assert_eq!(manifest.version, "0.1.0");
|
||||
assert!(manifest.tags.contains(&"default".to_string()));
|
||||
|
||||
let entry = manifest.to_registry_entry().unwrap();
|
||||
let entry = manifest.to_registry_entry();
|
||||
assert_eq!(entry.kind, ExtensionKind::WasmTool);
|
||||
}
|
||||
|
||||
@@ -347,7 +262,7 @@ mod tests {
|
||||
assert!(manifest.auth_summary.is_none());
|
||||
assert!(manifest.artifacts.is_empty());
|
||||
|
||||
let entry = manifest.to_registry_entry().unwrap();
|
||||
let entry = manifest.to_registry_entry();
|
||||
assert_eq!(entry.kind, ExtensionKind::WasmChannel);
|
||||
}
|
||||
|
||||
@@ -381,7 +296,6 @@ mod tests {
|
||||
fn test_manifest_kind_display() {
|
||||
assert_eq!(ManifestKind::Tool.to_string(), "tool");
|
||||
assert_eq!(ManifestKind::Channel.to_string(), "channel");
|
||||
assert_eq!(ManifestKind::McpServer.to_string(), "mcp_server");
|
||||
}
|
||||
|
||||
/// When a manifest has a download URL in artifacts, to_registry_entry()
|
||||
@@ -410,7 +324,7 @@ mod tests {
|
||||
}"#;
|
||||
|
||||
let manifest: ExtensionManifest = serde_json::from_str(json).expect("parse manifest");
|
||||
let entry = manifest.to_registry_entry().unwrap();
|
||||
let entry = manifest.to_registry_entry();
|
||||
|
||||
// Primary source should be WasmDownload
|
||||
assert!(
|
||||
@@ -460,7 +374,7 @@ mod tests {
|
||||
}"#;
|
||||
|
||||
let manifest: ExtensionManifest = serde_json::from_str(json).expect("parse manifest");
|
||||
let entry = manifest.to_registry_entry().unwrap();
|
||||
let entry = manifest.to_registry_entry();
|
||||
|
||||
assert!(
|
||||
matches!(&entry.source, ExtensionSource::WasmBuildable { .. }),
|
||||
@@ -491,7 +405,7 @@ mod tests {
|
||||
}"#;
|
||||
|
||||
let manifest: ExtensionManifest = serde_json::from_str(json).expect("parse manifest");
|
||||
let entry = manifest.to_registry_entry().unwrap();
|
||||
let entry = manifest.to_registry_entry();
|
||||
|
||||
assert!(
|
||||
matches!(&entry.source, ExtensionSource::WasmBuildable { .. }),
|
||||
@@ -502,89 +416,4 @@ mod tests {
|
||||
"Should have no fallback when already using WasmBuildable"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_mcp_server_manifest() {
|
||||
let json = r#"{
|
||||
"name": "notion",
|
||||
"display_name": "Notion",
|
||||
"kind": "mcp_server",
|
||||
"description": "Connect to Notion for reading and writing pages, databases, and comments",
|
||||
"keywords": ["notes", "wiki", "docs", "pages", "database"],
|
||||
"url": "https://mcp.notion.com/mcp",
|
||||
"auth": "dcr"
|
||||
}"#;
|
||||
|
||||
let manifest: ExtensionManifest = serde_json::from_str(json).expect("parse manifest");
|
||||
assert_eq!(manifest.name, "notion");
|
||||
assert_eq!(manifest.kind, ManifestKind::McpServer);
|
||||
assert!(manifest.version.is_none());
|
||||
assert!(manifest.source.is_none());
|
||||
assert_eq!(manifest.url.as_deref(), Some("https://mcp.notion.com/mcp"));
|
||||
assert_eq!(manifest.auth.as_deref(), Some("dcr"));
|
||||
|
||||
let entry = manifest.to_registry_entry().unwrap();
|
||||
assert_eq!(entry.kind, ExtensionKind::McpServer);
|
||||
assert!(
|
||||
matches!(&entry.source, ExtensionSource::McpUrl { url } if url == "https://mcp.notion.com/mcp")
|
||||
);
|
||||
assert!(matches!(&entry.auth_hint, AuthHint::Dcr));
|
||||
assert!(entry.fallback_source.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_server_oauth_pre_configured() {
|
||||
let json = r#"{
|
||||
"name": "custom-mcp",
|
||||
"display_name": "Custom MCP",
|
||||
"kind": "mcp_server",
|
||||
"description": "Custom MCP server",
|
||||
"keywords": [],
|
||||
"url": "https://mcp.example.com",
|
||||
"auth": "oauth_pre_configured:https://example.com/setup"
|
||||
}"#;
|
||||
|
||||
let manifest: ExtensionManifest = serde_json::from_str(json).expect("parse manifest");
|
||||
let entry = manifest.to_registry_entry().unwrap();
|
||||
|
||||
assert!(matches!(
|
||||
&entry.auth_hint,
|
||||
AuthHint::OAuthPreConfigured { setup_url } if setup_url == "https://example.com/setup"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_server_auth_none() {
|
||||
let json = r#"{
|
||||
"name": "local-mcp",
|
||||
"display_name": "Local MCP",
|
||||
"kind": "mcp_server",
|
||||
"description": "Local MCP server",
|
||||
"keywords": [],
|
||||
"url": "http://localhost:8080/mcp",
|
||||
"auth": "none"
|
||||
}"#;
|
||||
|
||||
let manifest: ExtensionManifest = serde_json::from_str(json).expect("parse manifest");
|
||||
let entry = manifest.to_registry_entry().unwrap();
|
||||
|
||||
assert!(matches!(&entry.auth_hint, AuthHint::None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_server_missing_url_returns_none() {
|
||||
let json = r#"{
|
||||
"name": "broken-mcp",
|
||||
"display_name": "Broken MCP",
|
||||
"kind": "mcp_server",
|
||||
"description": "MCP server with no URL",
|
||||
"keywords": []
|
||||
}"#;
|
||||
|
||||
let manifest: ExtensionManifest = serde_json::from_str(json).expect("parse manifest");
|
||||
assert!(
|
||||
manifest.to_registry_entry().is_none(),
|
||||
"MCP manifest without url should return None"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -214,7 +214,6 @@ fn is_disallowed_ipv4(v4: &Ipv4Addr) -> bool {
|
||||
|| v4.is_multicast()
|
||||
|| v4.is_unspecified()
|
||||
|| *v4 == Ipv4Addr::new(169, 254, 169, 254)
|
||||
|| (v4.octets()[0] == 100 && (v4.octets()[1] & 0xC0) == 64)
|
||||
}
|
||||
|
||||
fn is_disallowed_ip(ip: &IpAddr) -> bool {
|
||||
@@ -914,8 +913,6 @@ mod tests {
|
||||
assert!(is_disallowed_ip(&IpAddr::V4(Ipv4Addr::new(
|
||||
169, 254, 169, 254
|
||||
))));
|
||||
// Carrier-grade NAT
|
||||
assert!(is_disallowed_ip(&IpAddr::V4(Ipv4Addr::new(100, 64, 0, 1))));
|
||||
// Public
|
||||
assert!(!is_disallowed_ip(&IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8))));
|
||||
}
|
||||
|
||||
+121
-252
@@ -24,132 +24,6 @@ use crate::context::JobContext;
|
||||
use crate::db::Database;
|
||||
use crate::tools::tool::{ApprovalRequirement, Tool, ToolError, ToolOutput, require_str};
|
||||
|
||||
pub(crate) fn routine_create_parameters_schema() -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {
|
||||
"type": "string",
|
||||
"description": "Unique routine name, for example 'daily-pr-review'."
|
||||
},
|
||||
"description": {
|
||||
"type": "string",
|
||||
"description": "Short summary of what the routine is for."
|
||||
},
|
||||
"trigger_type": {
|
||||
"type": "string",
|
||||
"enum": ["cron", "event", "system_event", "manual"],
|
||||
"description": "When the routine fires: 'cron' for schedules, 'event' for incoming messages, 'system_event' for structured emitted events, or 'manual' for explicit runs."
|
||||
},
|
||||
"schedule": {
|
||||
"type": "string",
|
||||
"description": "Cron schedule for 'cron' triggers. Uses 6 fields: second minute hour day month weekday."
|
||||
},
|
||||
"event_pattern": {
|
||||
"type": "string",
|
||||
"description": "Regex matched against incoming message text for 'event' triggers, for example '^bug\\\\b'."
|
||||
},
|
||||
"event_channel": {
|
||||
"type": "string",
|
||||
"description": "Optional platform filter for 'event' triggers, for example 'telegram'. Omit to match any channel. Not a chat or thread ID."
|
||||
},
|
||||
"event_source": {
|
||||
"type": "string",
|
||||
"description": "Structured event source for 'system_event' triggers, for example 'github'."
|
||||
},
|
||||
"event_type": {
|
||||
"type": "string",
|
||||
"description": "Structured event type for 'system_event' triggers, for example 'issue.opened'."
|
||||
},
|
||||
"event_filters": {
|
||||
"type": "object",
|
||||
"properties": {},
|
||||
"additionalProperties": {
|
||||
"type": ["string", "number", "boolean"]
|
||||
},
|
||||
"description": "Optional exact-match payload filters for 'system_event' triggers. Values can be strings, numbers, or booleans."
|
||||
},
|
||||
"prompt": {
|
||||
"type": "string",
|
||||
"description": "Instructions for what the routine should do after it fires."
|
||||
},
|
||||
"context_paths": {
|
||||
"type": "array",
|
||||
"items": { "type": "string" },
|
||||
"description": "Workspace paths to load as extra context before running the routine."
|
||||
},
|
||||
"action_type": {
|
||||
"type": "string",
|
||||
"enum": ["lightweight", "full_job"],
|
||||
"description": "Execution mode: 'lightweight' for one LLM turn or 'full_job' for a multi-step job with tools."
|
||||
},
|
||||
"use_tools": {
|
||||
"type": "boolean",
|
||||
"description": "Enable safe tool use in 'lightweight' mode. Ignored for 'full_job'."
|
||||
},
|
||||
"max_tool_rounds": {
|
||||
"type": "integer",
|
||||
"description": "Maximum tool-call rounds in 'lightweight' mode when 'use_tools' is true."
|
||||
},
|
||||
"cooldown_secs": {
|
||||
"type": "integer",
|
||||
"description": "Minimum seconds between fires."
|
||||
},
|
||||
"tool_permissions": {
|
||||
"type": "array",
|
||||
"items": { "type": "string" },
|
||||
"description": "Pre-authorized tool names for 'full_job' routines."
|
||||
},
|
||||
"notify_channel": {
|
||||
"type": "string",
|
||||
"description": "Where routine output should be sent, for example 'telegram' or 'slack'. This does not control what triggers the routine."
|
||||
},
|
||||
"notify_user": {
|
||||
"type": "string",
|
||||
"description": "User or destination to notify, for example a username or chat ID."
|
||||
},
|
||||
"timezone": {
|
||||
"type": "string",
|
||||
"description": "IANA timezone used to evaluate 'cron' schedules, for example 'America/New_York'."
|
||||
}
|
||||
},
|
||||
"required": ["name", "trigger_type", "prompt"]
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn routine_update_parameters_schema() -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {
|
||||
"type": "string",
|
||||
"description": "Name of the routine to update."
|
||||
},
|
||||
"enabled": {
|
||||
"type": "boolean",
|
||||
"description": "Set to true to enable the routine or false to disable it."
|
||||
},
|
||||
"prompt": {
|
||||
"type": "string",
|
||||
"description": "Replace the routine instructions for what it should do after it fires."
|
||||
},
|
||||
"schedule": {
|
||||
"type": "string",
|
||||
"description": "New cron schedule for existing 'cron' routines only. This does not convert other trigger types."
|
||||
},
|
||||
"timezone": {
|
||||
"type": "string",
|
||||
"description": "New IANA timezone for existing 'cron' routines only, for example 'America/New_York'."
|
||||
},
|
||||
"description": {
|
||||
"type": "string",
|
||||
"description": "Replace the routine summary."
|
||||
}
|
||||
},
|
||||
"required": ["name"]
|
||||
})
|
||||
}
|
||||
|
||||
// ==================== routine_create ====================
|
||||
|
||||
pub struct RoutineCreateTool {
|
||||
@@ -176,7 +50,92 @@ impl Tool for RoutineCreateTool {
|
||||
}
|
||||
|
||||
fn parameters_schema(&self) -> serde_json::Value {
|
||||
routine_create_parameters_schema()
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {
|
||||
"type": "string",
|
||||
"description": "Unique name for the routine (e.g. 'daily-pr-review')"
|
||||
},
|
||||
"description": {
|
||||
"type": "string",
|
||||
"description": "What this routine does"
|
||||
},
|
||||
"trigger_type": {
|
||||
"type": "string",
|
||||
"enum": ["cron", "event", "system_event", "manual"],
|
||||
"description": "When the routine fires"
|
||||
},
|
||||
"schedule": {
|
||||
"type": "string",
|
||||
"description": "Cron expression (for cron trigger). E.g. '0 9 * * MON-FRI' for weekdays at 9am. Uses 6-field cron (sec min hour day month weekday)."
|
||||
},
|
||||
"event_pattern": {
|
||||
"type": "string",
|
||||
"description": "Regex pattern to match messages (for event trigger)"
|
||||
},
|
||||
"event_channel": {
|
||||
"type": "string",
|
||||
"description": "Optional channel filter for event trigger (e.g. 'telegram')"
|
||||
},
|
||||
"event_source": {
|
||||
"type": "string",
|
||||
"description": "Event source for system_event triggers (e.g. 'github')"
|
||||
},
|
||||
"event_type": {
|
||||
"type": "string",
|
||||
"description": "Event type for system_event triggers (e.g. 'issue.opened')"
|
||||
},
|
||||
"event_filters": {
|
||||
"type": "object",
|
||||
"description": "Optional exact-match filters against payload fields for system_event triggers. Values can be strings, numbers, or booleans."
|
||||
},
|
||||
"prompt": {
|
||||
"type": "string",
|
||||
"description": "The prompt/instructions for the routine"
|
||||
},
|
||||
"context_paths": {
|
||||
"type": "array",
|
||||
"items": { "type": "string" },
|
||||
"description": "Workspace paths to load as context (e.g. ['context/priorities.md'])"
|
||||
},
|
||||
"action_type": {
|
||||
"type": "string",
|
||||
"enum": ["lightweight", "full_job"],
|
||||
"description": "Execution mode: 'lightweight' (single LLM call, default) or 'full_job' (multi-turn with tools)"
|
||||
},
|
||||
"use_tools": {
|
||||
"type": "boolean",
|
||||
"description": "Enable tool access in lightweight mode (default: false). Only safe tools (no approval required) are available. Ignored for full_job mode."
|
||||
},
|
||||
"max_tool_rounds": {
|
||||
"type": "integer",
|
||||
"description": "Max tool call rounds in lightweight mode (default: 3). Only used when use_tools is true."
|
||||
},
|
||||
"cooldown_secs": {
|
||||
"type": "integer",
|
||||
"description": "Minimum seconds between fires (default: 300)"
|
||||
},
|
||||
"tool_permissions": {
|
||||
"type": "array",
|
||||
"items": { "type": "string" },
|
||||
"description": "Tool names pre-authorized for Always-approval tools in full_job mode (e.g. ['shell']). UnlessAutoApproved tools are automatically permitted in routines."
|
||||
},
|
||||
"notify_channel": {
|
||||
"type": "string",
|
||||
"description": "Channel to send results to (e.g. 'telegram', 'slack', 'tui'). Sets the default channel for message tool calls in routine jobs."
|
||||
},
|
||||
"notify_user": {
|
||||
"type": "string",
|
||||
"description": "User/target to notify (e.g. username, chat ID). Defaults to 'default'."
|
||||
},
|
||||
"timezone": {
|
||||
"type": "string",
|
||||
"description": "IANA timezone for cron schedule evaluation (e.g. 'America/New_York'). Defaults to UTC."
|
||||
}
|
||||
},
|
||||
"required": ["name", "trigger_type", "prompt"]
|
||||
})
|
||||
}
|
||||
|
||||
async fn execute(
|
||||
@@ -240,13 +199,9 @@ impl Tool for RoutineCreateTool {
|
||||
"event trigger requires 'event_pattern'".to_string(),
|
||||
)
|
||||
})?;
|
||||
// Validate regex with size limit to prevent ReDoS (issue #825)
|
||||
regex::RegexBuilder::new(pattern)
|
||||
.size_limit(64 * 1024)
|
||||
.build()
|
||||
.map_err(|e| {
|
||||
ToolError::InvalidParameters(format!("invalid or too complex regex: {e}"))
|
||||
})?;
|
||||
// Validate regex
|
||||
regex::Regex::new(pattern)
|
||||
.map_err(|e| ToolError::InvalidParameters(format!("invalid regex: {e}")))?;
|
||||
let channel = params
|
||||
.get("event_channel")
|
||||
.and_then(|v| v.as_str())
|
||||
@@ -523,13 +478,41 @@ impl Tool for RoutineUpdateTool {
|
||||
}
|
||||
|
||||
fn description(&self) -> &str {
|
||||
"Update an existing routine. Can change prompt, description, enabled state, or cron timing. \
|
||||
Pass the routine name and only the fields you want to change. \
|
||||
This does not convert one trigger type into another."
|
||||
"Update an existing routine. Can modify trigger, prompt, schedule, or toggle enabled state. \
|
||||
Pass the routine name and only the fields you want to change."
|
||||
}
|
||||
|
||||
fn parameters_schema(&self) -> serde_json::Value {
|
||||
routine_update_parameters_schema()
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {
|
||||
"type": "string",
|
||||
"description": "Name of the routine to update"
|
||||
},
|
||||
"enabled": {
|
||||
"type": "boolean",
|
||||
"description": "Enable or disable the routine"
|
||||
},
|
||||
"prompt": {
|
||||
"type": "string",
|
||||
"description": "New prompt/instructions"
|
||||
},
|
||||
"schedule": {
|
||||
"type": "string",
|
||||
"description": "New cron schedule (for cron triggers)"
|
||||
},
|
||||
"timezone": {
|
||||
"type": "string",
|
||||
"description": "IANA timezone for cron schedule (e.g. 'America/New_York'). Only valid for cron triggers."
|
||||
},
|
||||
"description": {
|
||||
"type": "string",
|
||||
"description": "New description"
|
||||
}
|
||||
},
|
||||
"required": ["name"]
|
||||
})
|
||||
}
|
||||
|
||||
async fn execute(
|
||||
@@ -970,117 +953,3 @@ impl Tool for EventEmitTool {
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{routine_create_parameters_schema, routine_update_parameters_schema};
|
||||
use crate::tools::validate_tool_schema;
|
||||
|
||||
fn property<'a>(schema: &'a serde_json::Value, name: &str) -> &'a serde_json::Value {
|
||||
schema
|
||||
.get("properties")
|
||||
.and_then(|props| props.get(name))
|
||||
.unwrap_or_else(|| panic!("missing schema property {name}"))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn routine_create_schema_exposes_all_trigger_and_delivery_fields() {
|
||||
let schema = routine_create_parameters_schema();
|
||||
let errors = validate_tool_schema(&schema, "routine_create");
|
||||
assert!(
|
||||
errors.is_empty(),
|
||||
"routine_create schema should validate cleanly: {errors:?}"
|
||||
);
|
||||
|
||||
for field in [
|
||||
"trigger_type",
|
||||
"schedule",
|
||||
"event_pattern",
|
||||
"event_channel",
|
||||
"event_source",
|
||||
"event_type",
|
||||
"event_filters",
|
||||
"action_type",
|
||||
"use_tools",
|
||||
"max_tool_rounds",
|
||||
"tool_permissions",
|
||||
"notify_channel",
|
||||
"notify_user",
|
||||
"timezone",
|
||||
] {
|
||||
let _ = property(&schema, field);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn routine_create_schema_descriptions_cover_event_trigger_gotchas() {
|
||||
let schema = routine_create_parameters_schema();
|
||||
|
||||
let trigger_type = property(&schema, "trigger_type")
|
||||
.get("description")
|
||||
.and_then(|value| value.as_str())
|
||||
.expect("trigger_type description");
|
||||
assert!(trigger_type.contains("incoming messages"));
|
||||
assert!(trigger_type.contains("structured emitted events"));
|
||||
|
||||
let event_pattern = property(&schema, "event_pattern")
|
||||
.get("description")
|
||||
.and_then(|value| value.as_str())
|
||||
.expect("event_pattern description");
|
||||
assert!(event_pattern.contains("incoming message text"));
|
||||
assert!(event_pattern.contains("^bug\\\\b"));
|
||||
|
||||
let event_channel = property(&schema, "event_channel")
|
||||
.get("description")
|
||||
.and_then(|value| value.as_str())
|
||||
.expect("event_channel description");
|
||||
assert!(event_channel.contains("Omit to match any channel"));
|
||||
assert!(event_channel.contains("Not a chat or thread ID"));
|
||||
|
||||
let notify_channel = property(&schema, "notify_channel")
|
||||
.get("description")
|
||||
.and_then(|value| value.as_str())
|
||||
.expect("notify_channel description");
|
||||
assert!(notify_channel.contains("does not control what triggers"));
|
||||
|
||||
let prompt = property(&schema, "prompt")
|
||||
.get("description")
|
||||
.and_then(|value| value.as_str())
|
||||
.expect("prompt description");
|
||||
assert!(prompt.contains("after it fires"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn routine_update_schema_exposes_supported_fields_and_limits() {
|
||||
let schema = routine_update_parameters_schema();
|
||||
let errors = validate_tool_schema(&schema, "routine_update");
|
||||
assert!(
|
||||
errors.is_empty(),
|
||||
"routine_update schema should validate cleanly: {errors:?}"
|
||||
);
|
||||
|
||||
for field in [
|
||||
"name",
|
||||
"enabled",
|
||||
"prompt",
|
||||
"schedule",
|
||||
"timezone",
|
||||
"description",
|
||||
] {
|
||||
let _ = property(&schema, field);
|
||||
}
|
||||
|
||||
let schedule = property(&schema, "schedule")
|
||||
.get("description")
|
||||
.and_then(|value| value.as_str())
|
||||
.expect("schedule description");
|
||||
assert!(schedule.contains("existing 'cron' routines only"));
|
||||
assert!(schedule.contains("does not convert other trigger types"));
|
||||
|
||||
let timezone = property(&schema, "timezone")
|
||||
.get("description")
|
||||
.and_then(|value| value.as_str())
|
||||
.expect("timezone description");
|
||||
assert!(timezone.contains("existing 'cron' routines only"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -247,11 +247,7 @@ fn resolve_timezone_for_output(
|
||||
params: &serde_json::Value,
|
||||
ctx: &JobContext,
|
||||
) -> Result<Option<(Tz, String)>, ToolError> {
|
||||
if let Some(name) = params
|
||||
.get("timezone")
|
||||
.and_then(|v| v.as_str())
|
||||
.filter(|s| !s.is_empty())
|
||||
{
|
||||
if let Some(name) = params.get("timezone").and_then(|v| v.as_str()) {
|
||||
let tz = parse_timezone(name)?;
|
||||
return Ok(Some((tz, tz.to_string())));
|
||||
}
|
||||
@@ -290,11 +286,7 @@ fn context_timezone(ctx: &JobContext) -> Result<Option<(Tz, String)>, ToolError>
|
||||
|
||||
fn optional_timezone(params: &serde_json::Value, keys: &[&str]) -> Result<Option<Tz>, ToolError> {
|
||||
for key in keys {
|
||||
if let Some(value) = params
|
||||
.get(*key)
|
||||
.and_then(|v| v.as_str())
|
||||
.filter(|s| !s.is_empty())
|
||||
{
|
||||
if let Some(value) = params.get(*key).and_then(|v| v.as_str()) {
|
||||
return parse_timezone(value).map(Some);
|
||||
}
|
||||
}
|
||||
@@ -542,48 +534,4 @@ mod tests {
|
||||
|
||||
assert_eq!(dt.to_rfc3339(), "2026-03-08T07:30:00+00:00");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_now_with_empty_timezone_string_does_not_error() {
|
||||
// LLMs sometimes pass "" for optional fields instead of omitting them.
|
||||
// Empty timezone should be treated as absent and fall back to UTC.
|
||||
let tool = TimeTool;
|
||||
let ctx = JobContext::with_user("test", "chat", "test");
|
||||
|
||||
let output = tool
|
||||
.execute(
|
||||
serde_json::json!({
|
||||
"operation": "now",
|
||||
"timezone": ""
|
||||
}),
|
||||
&ctx,
|
||||
)
|
||||
.await
|
||||
.expect("empty timezone string should not error");
|
||||
|
||||
assert!(output.result.get("iso").is_some(), "should have iso");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_convert_with_empty_from_timezone_string_does_not_error() {
|
||||
// LLMs sometimes pass "" for optional fields instead of omitting them.
|
||||
// Empty from_timezone should be treated as absent.
|
||||
let tool = TimeTool;
|
||||
let ctx = JobContext::with_user("test", "chat", "test");
|
||||
|
||||
let output = tool
|
||||
.execute(
|
||||
serde_json::json!({
|
||||
"operation": "convert",
|
||||
"timestamp": "2026-03-08T12:00:00Z",
|
||||
"to_timezone": "America/New_York",
|
||||
"from_timezone": ""
|
||||
}),
|
||||
&ctx,
|
||||
)
|
||||
.await
|
||||
.expect("empty from_timezone string should not error");
|
||||
|
||||
assert!(output.result.get("output").is_some(), "should have output");
|
||||
}
|
||||
}
|
||||
|
||||
+40
-60
@@ -18,44 +18,6 @@ use crate::cli::oauth_defaults::{self, OAUTH_CALLBACK_PORT};
|
||||
use crate::secrets::{CreateSecretParams, SecretsStore};
|
||||
use crate::tools::mcp::config::McpServerConfig;
|
||||
|
||||
/// Shared HTTP client for all OAuth/discovery requests.
|
||||
///
|
||||
/// Redirects are disabled for security (prevents redirect-based SSRF).
|
||||
/// Per-request timeouts can override the default via `.timeout()` on
|
||||
/// the request builder.
|
||||
fn oauth_http_client() -> Result<&'static reqwest::Client, AuthError> {
|
||||
static CLIENT: std::sync::OnceLock<Result<reqwest::Client, String>> =
|
||||
std::sync::OnceLock::new();
|
||||
CLIENT
|
||||
.get_or_init(|| {
|
||||
reqwest::Client::builder()
|
||||
.timeout(Duration::from_secs(30))
|
||||
.redirect(reqwest::redirect::Policy::none())
|
||||
.build()
|
||||
.map_err(|e| e.to_string())
|
||||
})
|
||||
.as_ref()
|
||||
.map_err(|e| AuthError::Http(e.clone()))
|
||||
}
|
||||
|
||||
/// Log a debug message when a discovery/auth response is a redirect.
|
||||
/// Helps users diagnose configuration issues when legitimate servers
|
||||
/// redirect and our no-redirect policy causes a failure.
|
||||
fn log_redirect_if_applicable(url: &str, response: &reqwest::Response) {
|
||||
if response.status().is_redirection() {
|
||||
let location = response
|
||||
.headers()
|
||||
.get("location")
|
||||
.and_then(|v| v.to_str().ok());
|
||||
tracing::debug!(
|
||||
"OAuth request to '{}' returned redirect {} -> {:?} (redirects disabled for security)",
|
||||
url,
|
||||
response.status(),
|
||||
location
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// OAuth authorization error.
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum AuthError {
|
||||
@@ -325,8 +287,10 @@ async fn validate_url_safe(url: &str) -> Result<(), AuthError> {
|
||||
)));
|
||||
}
|
||||
if scheme == "http" {
|
||||
if !crate::tools::mcp::config::is_localhost_url(url) {
|
||||
let host = parsed.host_str().unwrap_or("");
|
||||
let host = parsed.host_str().unwrap_or("");
|
||||
let is_localhost =
|
||||
host == "localhost" || host == "127.0.0.1" || host == "::1" || host == "[::1]";
|
||||
if !is_localhost {
|
||||
return Err(AuthError::DiscoveryFailed(format!(
|
||||
"HTTP is only allowed for localhost; use HTTPS for '{}'",
|
||||
host
|
||||
@@ -418,17 +382,18 @@ fn parse_resource_metadata_url(www_authenticate: &str) -> Option<String> {
|
||||
async fn fetch_resource_metadata(url: &str) -> Result<ProtectedResourceMetadata, AuthError> {
|
||||
validate_url_safe(url).await?;
|
||||
|
||||
let client = oauth_http_client()?;
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(Duration::from_secs(10))
|
||||
.redirect(reqwest::redirect::Policy::none())
|
||||
.build()
|
||||
.map_err(|e| AuthError::Http(e.to_string()))?;
|
||||
|
||||
let response = client
|
||||
.get(url)
|
||||
.timeout(Duration::from_secs(10))
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| AuthError::DiscoveryFailed(e.to_string()))?;
|
||||
|
||||
log_redirect_if_applicable(url, &response);
|
||||
|
||||
if !response.status().is_success() {
|
||||
return Err(AuthError::DiscoveryFailed(format!(
|
||||
"HTTP {}",
|
||||
@@ -446,19 +411,20 @@ async fn fetch_resource_metadata(url: &str) -> Result<ProtectedResourceMetadata,
|
||||
async fn discover_via_401(server_url: &str) -> Result<AuthorizationServerMetadata, AuthError> {
|
||||
validate_url_safe(server_url).await?;
|
||||
|
||||
let client = oauth_http_client()?;
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(Duration::from_secs(10))
|
||||
.redirect(reqwest::redirect::Policy::none())
|
||||
.build()
|
||||
.map_err(|e| AuthError::Http(e.to_string()))?;
|
||||
|
||||
let response = client
|
||||
.post(server_url)
|
||||
.timeout(Duration::from_secs(10))
|
||||
.header("Content-Type", "application/json")
|
||||
.body("{}")
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| AuthError::DiscoveryFailed(e.to_string()))?;
|
||||
|
||||
log_redirect_if_applicable(server_url, &response);
|
||||
|
||||
if response.status().as_u16() != 401 {
|
||||
return Err(AuthError::DiscoveryFailed(format!(
|
||||
"Expected 401, got {}",
|
||||
@@ -506,19 +472,20 @@ pub async fn discover_protected_resource(
|
||||
) -> Result<ProtectedResourceMetadata, AuthError> {
|
||||
validate_url_safe(server_url).await?;
|
||||
|
||||
let client = oauth_http_client()?;
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(Duration::from_secs(10))
|
||||
.redirect(reqwest::redirect::Policy::none())
|
||||
.build()
|
||||
.map_err(|e| AuthError::Http(e.to_string()))?;
|
||||
|
||||
let well_known_url = build_well_known_uri(server_url, "oauth-protected-resource")?;
|
||||
|
||||
let response = client
|
||||
.get(&well_known_url)
|
||||
.timeout(Duration::from_secs(10))
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| AuthError::DiscoveryFailed(e.to_string()))?;
|
||||
|
||||
log_redirect_if_applicable(&well_known_url, &response);
|
||||
|
||||
if !response.status().is_success() {
|
||||
return Err(AuthError::NotSupported);
|
||||
}
|
||||
@@ -535,19 +502,20 @@ pub async fn discover_authorization_server(
|
||||
) -> Result<AuthorizationServerMetadata, AuthError> {
|
||||
validate_url_safe(auth_server_url).await?;
|
||||
|
||||
let client = oauth_http_client()?;
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(Duration::from_secs(10))
|
||||
.redirect(reqwest::redirect::Policy::none())
|
||||
.build()
|
||||
.map_err(|e| AuthError::Http(e.to_string()))?;
|
||||
|
||||
let well_known_url = build_well_known_uri(auth_server_url, "oauth-authorization-server")?;
|
||||
|
||||
let response = client
|
||||
.get(&well_known_url)
|
||||
.timeout(Duration::from_secs(10))
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| AuthError::DiscoveryFailed(e.to_string()))?;
|
||||
|
||||
log_redirect_if_applicable(&well_known_url, &response);
|
||||
|
||||
if !response.status().is_success() {
|
||||
return Err(AuthError::DiscoveryFailed(format!(
|
||||
"HTTP {}",
|
||||
@@ -627,7 +595,11 @@ pub async fn register_client(
|
||||
) -> Result<ClientRegistrationResponse, AuthError> {
|
||||
validate_url_safe(registration_endpoint).await?;
|
||||
|
||||
let client = oauth_http_client()?;
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(Duration::from_secs(30))
|
||||
.redirect(reqwest::redirect::Policy::none())
|
||||
.build()
|
||||
.map_err(|e| AuthError::Http(e.to_string()))?;
|
||||
|
||||
let request = ClientRegistrationRequest {
|
||||
client_name: "IronClaw".to_string(),
|
||||
@@ -841,7 +813,7 @@ pub fn build_authorization_url(
|
||||
if let Some(pkce) = pkce {
|
||||
url.push_str(&format!(
|
||||
"&code_challenge={}&code_challenge_method=S256",
|
||||
urlencoding::encode(&pkce.challenge)
|
||||
pkce.challenge
|
||||
));
|
||||
}
|
||||
|
||||
@@ -891,7 +863,11 @@ pub async fn exchange_code_for_token(
|
||||
) -> Result<AccessToken, AuthError> {
|
||||
validate_url_safe(token_url).await?;
|
||||
|
||||
let client = oauth_http_client()?;
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(Duration::from_secs(30))
|
||||
.redirect(reqwest::redirect::Policy::none())
|
||||
.build()
|
||||
.map_err(|e| AuthError::Http(e.to_string()))?;
|
||||
|
||||
let mut params = vec![
|
||||
("grant_type", "authorization_code".to_string()),
|
||||
@@ -1078,7 +1054,11 @@ pub async fn refresh_access_token(
|
||||
|
||||
validate_url_safe(&token_url).await?;
|
||||
|
||||
let client = oauth_http_client()?;
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(Duration::from_secs(30))
|
||||
.redirect(reqwest::redirect::Policy::none())
|
||||
.build()
|
||||
.map_err(|e| AuthError::Http(e.to_string()))?;
|
||||
|
||||
// Compute canonical resource URI for RFC 8707
|
||||
let resource = canonical_resource_uri(&server_config.url);
|
||||
|
||||
+63
-205
@@ -5,7 +5,7 @@
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
|
||||
|
||||
use async_trait::async_trait;
|
||||
use tokio::sync::RwLock;
|
||||
@@ -58,10 +58,9 @@ pub struct McpClient {
|
||||
/// Custom headers to include in every request.
|
||||
custom_headers: HashMap<String, String>,
|
||||
|
||||
/// Ensures the MCP initialize handshake runs exactly once.
|
||||
/// Uses `OnceCell` to serialize concurrent callers so only one
|
||||
/// actually sends the request; subsequent calls return immediately.
|
||||
initialized: tokio::sync::OnceCell<InitializeResult>,
|
||||
/// Whether the MCP initialize handshake has completed.
|
||||
/// Used as a local idempotency guard when no session_manager is present.
|
||||
initialized: AtomicBool,
|
||||
}
|
||||
|
||||
impl McpClient {
|
||||
@@ -84,7 +83,7 @@ impl McpClient {
|
||||
user_id: "default".to_string(),
|
||||
server_config: None,
|
||||
custom_headers: HashMap::new(),
|
||||
initialized: tokio::sync::OnceCell::new(),
|
||||
initialized: AtomicBool::new(false),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -107,7 +106,7 @@ impl McpClient {
|
||||
user_id: "default".to_string(),
|
||||
server_config: None,
|
||||
custom_headers: HashMap::new(),
|
||||
initialized: tokio::sync::OnceCell::new(),
|
||||
initialized: AtomicBool::new(false),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -115,24 +114,20 @@ impl McpClient {
|
||||
///
|
||||
/// Use this when you have an `McpServerConfig` with custom headers but no OAuth.
|
||||
/// The config must use HTTP transport (the default); for stdio/UDS use `new_with_transport`.
|
||||
///
|
||||
/// Returns an error if the config uses a non-HTTP transport.
|
||||
pub fn new_with_config(config: McpServerConfig) -> Result<Self, ToolError> {
|
||||
if !matches!(
|
||||
config.effective_transport(),
|
||||
crate::tools::mcp::config::EffectiveTransport::Http
|
||||
) {
|
||||
return Err(ToolError::InvalidParameters(
|
||||
"new_with_config only supports HTTP transport; use new_with_transport for stdio/UDS"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
pub fn new_with_config(config: McpServerConfig) -> Self {
|
||||
assert!(
|
||||
matches!(
|
||||
config.effective_transport(),
|
||||
crate::tools::mcp::config::EffectiveTransport::Http
|
||||
),
|
||||
"new_with_config only supports HTTP transport; use new_with_transport for stdio/UDS"
|
||||
);
|
||||
let transport = Arc::new(HttpMcpTransport::new(
|
||||
config.url.clone(),
|
||||
config.name.clone(),
|
||||
));
|
||||
|
||||
Ok(Self {
|
||||
Self {
|
||||
transport,
|
||||
server_url: config.url.clone(),
|
||||
server_name: config.name.clone(),
|
||||
@@ -142,9 +137,9 @@ impl McpClient {
|
||||
secrets: None,
|
||||
user_id: "default".to_string(),
|
||||
custom_headers: config.headers.clone(),
|
||||
initialized: tokio::sync::OnceCell::new(),
|
||||
initialized: AtomicBool::new(false),
|
||||
server_config: Some(config),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
/// Create a new authenticated MCP client.
|
||||
@@ -174,7 +169,7 @@ impl McpClient {
|
||||
user_id: user_id.into(),
|
||||
server_config: Some(config),
|
||||
custom_headers,
|
||||
initialized: tokio::sync::OnceCell::new(),
|
||||
initialized: AtomicBool::new(false),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -210,7 +205,7 @@ impl McpClient {
|
||||
user_id: user_id.into(),
|
||||
server_config,
|
||||
custom_headers,
|
||||
initialized: tokio::sync::OnceCell::new(),
|
||||
initialized: AtomicBool::new(false),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -341,64 +336,53 @@ impl McpClient {
|
||||
}
|
||||
|
||||
/// Initialize the connection to the MCP server.
|
||||
///
|
||||
/// Uses `OnceCell` to guarantee that exactly one caller performs the
|
||||
/// handshake, even under concurrent access. Subsequent calls return
|
||||
/// immediately.
|
||||
pub async fn initialize(&self) -> Result<InitializeResult, ToolError> {
|
||||
let result = self
|
||||
.initialized
|
||||
.get_or_try_init(|| async {
|
||||
if let Some(ref session_manager) = self.session_manager
|
||||
&& session_manager.is_initialized(&self.server_name).await
|
||||
{
|
||||
return Ok(InitializeResult::default());
|
||||
}
|
||||
if let Some(ref session_manager) = self.session_manager {
|
||||
session_manager
|
||||
.get_or_create(&self.server_name, &self.server_url)
|
||||
.await;
|
||||
}
|
||||
// Fast path: already initialized (local flag or session manager)
|
||||
if self.initialized.load(Ordering::Relaxed) {
|
||||
return Ok(InitializeResult::default());
|
||||
}
|
||||
if let Some(ref session_manager) = self.session_manager
|
||||
&& session_manager.is_initialized(&self.server_name).await
|
||||
{
|
||||
self.initialized.store(true, Ordering::Relaxed);
|
||||
return Ok(InitializeResult::default());
|
||||
}
|
||||
if let Some(ref session_manager) = self.session_manager {
|
||||
session_manager
|
||||
.get_or_create(&self.server_name, &self.server_url)
|
||||
.await;
|
||||
}
|
||||
|
||||
let request = McpRequest::initialize(self.next_request_id());
|
||||
let response = self.send_request(request).await?;
|
||||
let request = McpRequest::initialize(self.next_request_id());
|
||||
let response = self.send_request(request).await?;
|
||||
|
||||
if let Some(error) = response.error {
|
||||
return Err(ToolError::ExternalService(format!(
|
||||
"MCP initialization error: {} (code {})",
|
||||
error.message, error.code
|
||||
)));
|
||||
}
|
||||
if let Some(error) = response.error {
|
||||
return Err(ToolError::ExternalService(format!(
|
||||
"MCP initialization error: {} (code {})",
|
||||
error.message, error.code
|
||||
)));
|
||||
}
|
||||
|
||||
let init_result: InitializeResult = response
|
||||
.result
|
||||
.ok_or_else(|| {
|
||||
ToolError::ExternalService("No result in initialize response".to_string())
|
||||
})
|
||||
.and_then(|r| {
|
||||
serde_json::from_value(r).map_err(|e| {
|
||||
ToolError::ExternalService(format!("Invalid initialize result: {}", e))
|
||||
})
|
||||
})?;
|
||||
|
||||
if let Some(ref session_manager) = self.session_manager {
|
||||
session_manager.mark_initialized(&self.server_name).await;
|
||||
}
|
||||
|
||||
let notification = McpRequest::initialized_notification();
|
||||
if let Err(e) = self.send_request(notification).await {
|
||||
tracing::debug!(
|
||||
"Failed to send initialized notification to '{}': {}",
|
||||
self.server_name,
|
||||
e
|
||||
);
|
||||
}
|
||||
|
||||
Ok(init_result)
|
||||
let result: InitializeResult = response
|
||||
.result
|
||||
.ok_or_else(|| {
|
||||
ToolError::ExternalService("No result in initialize response".to_string())
|
||||
})
|
||||
.await?;
|
||||
.and_then(|r| {
|
||||
serde_json::from_value(r).map_err(|e| {
|
||||
ToolError::ExternalService(format!("Invalid initialize result: {}", e))
|
||||
})
|
||||
})?;
|
||||
|
||||
Ok(result.clone())
|
||||
if let Some(ref session_manager) = self.session_manager {
|
||||
session_manager.mark_initialized(&self.server_name).await;
|
||||
}
|
||||
self.initialized.store(true, Ordering::Relaxed);
|
||||
|
||||
let notification = McpRequest::initialized_notification();
|
||||
let _ = self.send_request(notification).await;
|
||||
|
||||
Ok(result)
|
||||
}
|
||||
|
||||
/// List available tools from the MCP server.
|
||||
@@ -487,11 +471,6 @@ impl McpClient {
|
||||
}
|
||||
}
|
||||
|
||||
/// Clone the client, resetting the tools cache and initialization state.
|
||||
/// The cloned client shares the same transport and session manager, so
|
||||
/// re-initialization will short-circuit via the session manager check if
|
||||
/// the source was already initialized. The `next_id` counter is copied
|
||||
/// so that cloned clients continue with monotonically increasing IDs.
|
||||
impl Clone for McpClient {
|
||||
fn clone(&self) -> Self {
|
||||
Self {
|
||||
@@ -505,7 +484,7 @@ impl Clone for McpClient {
|
||||
user_id: self.user_id.clone(),
|
||||
server_config: self.server_config.clone(),
|
||||
custom_headers: self.custom_headers.clone(),
|
||||
initialized: tokio::sync::OnceCell::new(),
|
||||
initialized: AtomicBool::new(self.initialized.load(Ordering::Relaxed)),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -728,7 +707,7 @@ mod tests {
|
||||
headers.insert("X-Custom".to_string(), "value".to_string());
|
||||
|
||||
let config = McpServerConfig::new("test", "http://localhost:8080").with_headers(headers);
|
||||
let client = McpClient::new_with_config(config.clone()).expect("HTTP config should work");
|
||||
let client = McpClient::new_with_config(config.clone());
|
||||
|
||||
assert_eq!(client.server_name(), "test");
|
||||
assert_eq!(client.server_url(), "http://localhost:8080");
|
||||
@@ -740,7 +719,7 @@ mod tests {
|
||||
#[test]
|
||||
fn test_new_with_config_no_headers() {
|
||||
let config = McpServerConfig::new("bare", "http://localhost:9090");
|
||||
let client = McpClient::new_with_config(config).expect("HTTP config should work");
|
||||
let client = McpClient::new_with_config(config);
|
||||
|
||||
assert_eq!(client.server_name(), "bare");
|
||||
assert!(client.custom_headers.is_empty());
|
||||
@@ -992,125 +971,4 @@ mod tests {
|
||||
assert_eq!(obj.len(), 1);
|
||||
assert!(obj["outer"]["inner"].is_null());
|
||||
}
|
||||
|
||||
// --- Issue 1 regression: new_with_config rejects non-HTTP transport ---
|
||||
|
||||
#[test]
|
||||
fn test_new_with_config_rejects_stdio_transport() {
|
||||
let config = McpServerConfig::new_stdio(
|
||||
"stdio-server",
|
||||
"echo",
|
||||
vec!["hello".to_string()],
|
||||
HashMap::new(),
|
||||
);
|
||||
let result = McpClient::new_with_config(config);
|
||||
let err = result
|
||||
.err()
|
||||
.expect("stdio config must be rejected")
|
||||
.to_string();
|
||||
assert!(
|
||||
err.contains("new_with_config only supports HTTP"),
|
||||
"error should explain the restriction: {}",
|
||||
err
|
||||
);
|
||||
}
|
||||
|
||||
// --- Issue 13: McpToolWrapper unit tests ---
|
||||
|
||||
fn make_test_mcp_tool(destructive: bool) -> McpTool {
|
||||
use crate::tools::mcp::protocol::McpToolAnnotations;
|
||||
McpTool {
|
||||
name: "do_thing".to_string(),
|
||||
description: "Does a thing".to_string(),
|
||||
input_schema: serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"input": {"type": "string"}
|
||||
}
|
||||
}),
|
||||
annotations: if destructive {
|
||||
Some(McpToolAnnotations {
|
||||
destructive_hint: true,
|
||||
side_effects_hint: false,
|
||||
read_only_hint: false,
|
||||
execution_time_hint: None,
|
||||
})
|
||||
} else {
|
||||
None
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_tool_wrapper_name_is_prefixed() {
|
||||
let client = Arc::new(McpClient::new("http://localhost:8080"));
|
||||
let wrapper = McpToolWrapper {
|
||||
tool: make_test_mcp_tool(false),
|
||||
prefixed_name: "mcp__myserver__do_thing".to_string(),
|
||||
client,
|
||||
};
|
||||
assert_eq!(wrapper.name(), "mcp__myserver__do_thing");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_tool_wrapper_description() {
|
||||
let client = Arc::new(McpClient::new("http://localhost:8080"));
|
||||
let wrapper = McpToolWrapper {
|
||||
tool: make_test_mcp_tool(false),
|
||||
prefixed_name: "mcp__s__do_thing".to_string(),
|
||||
client,
|
||||
};
|
||||
assert_eq!(wrapper.description(), "Does a thing");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_tool_wrapper_parameters_schema() {
|
||||
let client = Arc::new(McpClient::new("http://localhost:8080"));
|
||||
let wrapper = McpToolWrapper {
|
||||
tool: make_test_mcp_tool(false),
|
||||
prefixed_name: "mcp__s__do_thing".to_string(),
|
||||
client,
|
||||
};
|
||||
let schema = wrapper.parameters_schema();
|
||||
assert_eq!(schema["type"], "object");
|
||||
assert!(schema["properties"]["input"].is_object());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_tool_wrapper_requires_sanitization() {
|
||||
let client = Arc::new(McpClient::new("http://localhost:8080"));
|
||||
let wrapper = McpToolWrapper {
|
||||
tool: make_test_mcp_tool(false),
|
||||
prefixed_name: "mcp__s__do_thing".to_string(),
|
||||
client,
|
||||
};
|
||||
assert!(
|
||||
wrapper.requires_sanitization(),
|
||||
"MCP tools should always require sanitization"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_tool_wrapper_approval_destructive() {
|
||||
let client = Arc::new(McpClient::new("http://localhost:8080"));
|
||||
let wrapper = McpToolWrapper {
|
||||
tool: make_test_mcp_tool(true),
|
||||
prefixed_name: "mcp__s__do_thing".to_string(),
|
||||
client,
|
||||
};
|
||||
let approval = wrapper.requires_approval(&serde_json::json!({}));
|
||||
assert_eq!(approval, ApprovalRequirement::UnlessAutoApproved);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_tool_wrapper_approval_non_destructive() {
|
||||
let client = Arc::new(McpClient::new("http://localhost:8080"));
|
||||
let wrapper = McpToolWrapper {
|
||||
tool: make_test_mcp_tool(false),
|
||||
prefixed_name: "mcp__s__do_thing".to_string(),
|
||||
client,
|
||||
};
|
||||
let approval = wrapper.requires_approval(&serde_json::json!({}));
|
||||
assert_eq!(approval, ApprovalRequirement::Never);
|
||||
}
|
||||
}
|
||||
|
||||
+6
-38
@@ -163,8 +163,10 @@ impl McpServerConfig {
|
||||
}
|
||||
|
||||
// Remote servers must use HTTPS (localhost is allowed for development)
|
||||
let is_localhost = is_localhost_url(&self.url);
|
||||
if !is_localhost && !self.url.to_lowercase().starts_with("https://") {
|
||||
let url_lower = self.url.to_lowercase();
|
||||
let is_localhost =
|
||||
url_lower.contains("localhost") || url_lower.contains("127.0.0.1");
|
||||
if !is_localhost && !url_lower.starts_with("https://") {
|
||||
return Err(ConfigError::InvalidConfig {
|
||||
reason: "Remote MCP servers must use HTTPS".to_string(),
|
||||
});
|
||||
@@ -440,12 +442,7 @@ pub async fn save_mcp_servers_to(
|
||||
}
|
||||
|
||||
let content = serde_json::to_string_pretty(config)?;
|
||||
|
||||
// Write to a temporary file first, then atomically rename to avoid
|
||||
// corrupting the config if the process crashes during the write.
|
||||
let tmp_path = path.with_extension("json.tmp");
|
||||
fs::write(&tmp_path, content).await?;
|
||||
fs::rename(&tmp_path, path).await?;
|
||||
fs::write(path, content).await?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -573,7 +570,7 @@ pub async fn remove_mcp_server_db(
|
||||
///
|
||||
/// Uses `url::Url` for proper parsing so edge cases (IPv6, userinfo, ports)
|
||||
/// are handled correctly without manual string splitting.
|
||||
pub(crate) fn is_localhost_url(url: &str) -> bool {
|
||||
fn is_localhost_url(url: &str) -> bool {
|
||||
let Ok(parsed) = url::Url::parse(url) else {
|
||||
return false;
|
||||
};
|
||||
@@ -1128,33 +1125,4 @@ mod tests {
|
||||
assert!(parsed.transport.is_none());
|
||||
assert_eq!(parsed.headers.get("X-Custom").unwrap(), "value");
|
||||
}
|
||||
|
||||
// --- Issue 3 regression: is_localhost_url rejects attacker subdomains ---
|
||||
|
||||
#[test]
|
||||
fn test_is_localhost_url_rejects_attacker_subdomain() {
|
||||
// Before the fix, url.contains("localhost") matched this.
|
||||
assert!(
|
||||
!is_localhost_url("http://evil.localhost.attacker.com:8080/mcp"),
|
||||
"attacker subdomain containing 'localhost' must not be treated as local"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_localhost_url_accepts_real_localhost() {
|
||||
assert!(is_localhost_url("http://localhost:8080/mcp"));
|
||||
assert!(is_localhost_url("https://localhost/path"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_localhost_url_accepts_loopback_ip() {
|
||||
assert!(is_localhost_url("http://127.0.0.1:3000"));
|
||||
assert!(is_localhost_url("http://[::1]:3000"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_localhost_url_rejects_remote() {
|
||||
assert!(!is_localhost_url("https://mcp.example.com"));
|
||||
assert!(!is_localhost_url("http://192.168.1.1:8080"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,8 +18,6 @@ pub enum McpFactoryError {
|
||||
UnixConnect { name: String, reason: String },
|
||||
#[error("Unix socket transport is not supported on this platform (server '{name}')")]
|
||||
UnixNotSupported { name: String },
|
||||
#[error("Invalid configuration for MCP server '{name}': {reason}")]
|
||||
InvalidConfig { name: String, reason: String },
|
||||
}
|
||||
|
||||
/// Create an `McpClient` from a server configuration, dispatching on the
|
||||
@@ -91,18 +89,10 @@ pub async fn create_client_from_config(
|
||||
))
|
||||
} else {
|
||||
Ok(McpClient::new_with_config(server)
|
||||
.map_err(|e| McpFactoryError::InvalidConfig {
|
||||
name: server_name.clone(),
|
||||
reason: e.to_string(),
|
||||
})?
|
||||
.with_session_manager(Arc::clone(session_manager)))
|
||||
}
|
||||
} else {
|
||||
Ok(McpClient::new_with_config(server)
|
||||
.map_err(|e| McpFactoryError::InvalidConfig {
|
||||
name: server_name,
|
||||
reason: e.to_string(),
|
||||
})?
|
||||
.with_session_manager(Arc::clone(session_manager)))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -139,7 +139,7 @@ impl McpTransport for HttpMcpTransport {
|
||||
.to_string();
|
||||
|
||||
if content_type.contains("text/event-stream") {
|
||||
self.parse_sse_response(response, request.id).await
|
||||
self.parse_sse_response(response).await
|
||||
} else {
|
||||
response.json().await.map_err(|e| {
|
||||
ToolError::ExternalService(format!(
|
||||
@@ -161,14 +161,11 @@ impl McpTransport for HttpMcpTransport {
|
||||
}
|
||||
|
||||
impl HttpMcpTransport {
|
||||
/// Parse a Server-Sent Events response, returning the JSON-RPC response
|
||||
/// whose `id` matches `request_id`. Non-matching events (e.g. server
|
||||
/// notifications or progress updates) are skipped so that the caller
|
||||
/// receives the actual result for its request.
|
||||
/// Parse a Server-Sent Events response, returning the first valid JSON-RPC
|
||||
/// `data:` line as an [`McpResponse`].
|
||||
async fn parse_sse_response(
|
||||
&self,
|
||||
response: reqwest::Response,
|
||||
request_id: Option<u64>,
|
||||
) -> Result<McpResponse, ToolError> {
|
||||
use futures::StreamExt;
|
||||
|
||||
@@ -205,10 +202,9 @@ impl HttpMcpTransport {
|
||||
remaining_start = i + 1;
|
||||
|
||||
if let Some(json_str) = line.strip_prefix("data: ")
|
||||
&& let Ok(resp) = serde_json::from_str::<McpResponse>(json_str)
|
||||
&& resp.id == request_id
|
||||
&& let Ok(response) = serde_json::from_str::<McpResponse>(json_str)
|
||||
{
|
||||
return Ok(resp);
|
||||
return Ok(response);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -220,15 +216,14 @@ impl HttpMcpTransport {
|
||||
|
||||
// Process any remaining data without a trailing newline.
|
||||
if let Some(json_str) = buffer.strip_prefix("data: ")
|
||||
&& let Ok(resp) = serde_json::from_str::<McpResponse>(json_str.trim())
|
||||
&& resp.id == request_id
|
||||
&& let Ok(response) = serde_json::from_str::<McpResponse>(json_str.trim())
|
||||
{
|
||||
return Ok(resp);
|
||||
return Ok(response);
|
||||
}
|
||||
|
||||
Err(ToolError::ExternalService(format!(
|
||||
"[{}] No matching response (id={:?}) in SSE stream",
|
||||
self.server_name, request_id
|
||||
"[{}] No valid data in SSE response: {}",
|
||||
self.server_name, buffer
|
||||
)))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,7 +14,7 @@ use tokio::sync::{Mutex, oneshot};
|
||||
use tokio::task::JoinHandle;
|
||||
|
||||
use crate::tools::mcp::protocol::{McpRequest, McpResponse};
|
||||
use crate::tools::mcp::transport::{McpTransport, spawn_jsonrpc_reader, stream_transport_send};
|
||||
use crate::tools::mcp::transport::{McpTransport, spawn_jsonrpc_reader, write_jsonrpc_line};
|
||||
use crate::tools::tool::ToolError;
|
||||
|
||||
/// MCP transport that communicates with a child process over stdin/stdout.
|
||||
@@ -118,14 +118,63 @@ impl McpTransport for StdioMcpTransport {
|
||||
request: &McpRequest,
|
||||
_headers: &HashMap<String, String>,
|
||||
) -> Result<McpResponse, ToolError> {
|
||||
stream_transport_send(
|
||||
&self.stdin,
|
||||
&self.pending,
|
||||
request,
|
||||
&self.server_name,
|
||||
Duration::from_secs(30),
|
||||
)
|
||||
.await
|
||||
// JSON-RPC notifications (no id) are fire-and-forget: the server
|
||||
// will not send a response, so we must not wait for one.
|
||||
if request.id.is_none() {
|
||||
let mut stdin = self.stdin.lock().await;
|
||||
write_jsonrpc_line(&mut *stdin, request).await?;
|
||||
return Ok(McpResponse {
|
||||
jsonrpc: "2.0".to_string(),
|
||||
id: None,
|
||||
result: None,
|
||||
error: None,
|
||||
});
|
||||
}
|
||||
|
||||
let id = request.id.unwrap_or(0);
|
||||
let (tx, rx) = oneshot::channel();
|
||||
|
||||
// Register the pending response handler before writing the request,
|
||||
// so we don't miss a fast response from the child.
|
||||
{
|
||||
let mut pending = self.pending.lock().await;
|
||||
pending.insert(id, tx);
|
||||
}
|
||||
|
||||
// Write the request to stdin.
|
||||
{
|
||||
let mut stdin = self.stdin.lock().await;
|
||||
if let Err(e) = write_jsonrpc_line(&mut *stdin, request).await {
|
||||
// Remove the pending entry on write failure.
|
||||
let mut pending = self.pending.lock().await;
|
||||
pending.remove(&id);
|
||||
return Err(e);
|
||||
}
|
||||
}
|
||||
|
||||
// Wait for the response with a timeout.
|
||||
let timeout = Duration::from_secs(30);
|
||||
match tokio::time::timeout(timeout, rx).await {
|
||||
Ok(Ok(response)) => Ok(response),
|
||||
Ok(Err(_)) => {
|
||||
// Sender was dropped (reader task ended). Clean up pending entry.
|
||||
let mut pending = self.pending.lock().await;
|
||||
pending.remove(&id);
|
||||
Err(ToolError::ExternalService(format!(
|
||||
"[{}] MCP server closed connection before responding to request {:?}",
|
||||
self.server_name, request.id
|
||||
)))
|
||||
}
|
||||
Err(_) => {
|
||||
// Timeout: remove the pending entry.
|
||||
let mut pending = self.pending.lock().await;
|
||||
pending.remove(&id);
|
||||
Err(ToolError::ExternalService(format!(
|
||||
"[{}] Timeout waiting for response to request {:?} after {:?}",
|
||||
self.server_name, request.id, timeout
|
||||
)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn shutdown(&self) -> Result<(), ToolError> {
|
||||
|
||||
+1
-105
@@ -97,13 +97,7 @@ pub fn spawn_jsonrpc_reader<R: AsyncBufRead + Unpin + Send + 'static>(
|
||||
}
|
||||
};
|
||||
|
||||
let Some(id) = response.id else {
|
||||
tracing::debug!(
|
||||
"[{}] Received JSON-RPC notification (no id), skipping dispatch",
|
||||
server_name
|
||||
);
|
||||
continue;
|
||||
};
|
||||
let id = response.id.unwrap_or(0);
|
||||
let mut map = pending.lock().await;
|
||||
if let Some(tx) = map.remove(&id) {
|
||||
// Ignore send error — the receiver may have been dropped (timeout).
|
||||
@@ -121,76 +115,6 @@ pub fn spawn_jsonrpc_reader<R: AsyncBufRead + Unpin + Send + 'static>(
|
||||
})
|
||||
}
|
||||
|
||||
/// Send a JSON-RPC request over a stream-based transport (stdio / unix socket).
|
||||
///
|
||||
/// Handles notification fire-and-forget, pending response registration,
|
||||
/// write, timeout, and cleanup. Used by both [`StdioMcpTransport`] and
|
||||
/// [`UnixMcpTransport`] to avoid duplicating the send logic.
|
||||
pub(crate) async fn stream_transport_send<W: AsyncWrite + Unpin>(
|
||||
writer: &Mutex<W>,
|
||||
pending: &Mutex<HashMap<u64, oneshot::Sender<McpResponse>>>,
|
||||
request: &McpRequest,
|
||||
server_name: &str,
|
||||
timeout_duration: std::time::Duration,
|
||||
) -> Result<McpResponse, ToolError> {
|
||||
// JSON-RPC notifications (no id) are fire-and-forget: the server
|
||||
// will not send a response, so we must not wait for one.
|
||||
if request.id.is_none() {
|
||||
let mut w = writer.lock().await;
|
||||
write_jsonrpc_line(&mut *w, request).await?;
|
||||
return Ok(McpResponse {
|
||||
jsonrpc: "2.0".to_string(),
|
||||
id: None,
|
||||
result: None,
|
||||
error: None,
|
||||
});
|
||||
}
|
||||
|
||||
let id = request.id.unwrap_or(0);
|
||||
let (tx, rx) = oneshot::channel();
|
||||
|
||||
// Register the pending response handler before writing the request,
|
||||
// so we don't miss a fast response from the server.
|
||||
{
|
||||
let mut map = pending.lock().await;
|
||||
map.insert(id, tx);
|
||||
}
|
||||
|
||||
// Write the request.
|
||||
{
|
||||
let mut w = writer.lock().await;
|
||||
if let Err(e) = write_jsonrpc_line(&mut *w, request).await {
|
||||
// Remove the pending entry on write failure.
|
||||
let mut map = pending.lock().await;
|
||||
map.remove(&id);
|
||||
return Err(e);
|
||||
}
|
||||
}
|
||||
|
||||
// Wait for the response with a timeout.
|
||||
match tokio::time::timeout(timeout_duration, rx).await {
|
||||
Ok(Ok(response)) => Ok(response),
|
||||
Ok(Err(_)) => {
|
||||
// Sender was dropped (reader task ended). Clean up pending entry.
|
||||
let mut map = pending.lock().await;
|
||||
map.remove(&id);
|
||||
Err(ToolError::ExternalService(format!(
|
||||
"[{}] MCP server closed connection before responding to request {:?}",
|
||||
server_name, request.id
|
||||
)))
|
||||
}
|
||||
Err(_) => {
|
||||
// Timeout: remove the pending entry.
|
||||
let mut map = pending.lock().await;
|
||||
map.remove(&id);
|
||||
Err(ToolError::ExternalService(format!(
|
||||
"[{}] Timeout waiting for response to request {:?} after {:?}",
|
||||
server_name, request.id, timeout_duration
|
||||
)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -269,32 +193,4 @@ mod tests {
|
||||
|
||||
handle.await.expect("reader task should finish");
|
||||
}
|
||||
|
||||
/// Issue 9 regression: a JSON-RPC notification (no id) must not resolve
|
||||
/// a pending request keyed by id 0 (the old `unwrap_or(0)` default).
|
||||
#[tokio::test]
|
||||
async fn test_notification_does_not_resolve_pending_id_zero() {
|
||||
// A notification response (no id), followed by a proper response for id 0.
|
||||
let notification = r#"{"jsonrpc":"2.0","method":"notifications/progress","params":{}}"#;
|
||||
let real_response = r#"{"jsonrpc":"2.0","id":0,"result":{"ok":true}}"#;
|
||||
let input = format!("{notification}\n{real_response}\n");
|
||||
|
||||
let reader = std::io::Cursor::new(input.into_bytes());
|
||||
let pending: Arc<Mutex<HashMap<u64, oneshot::Sender<McpResponse>>>> =
|
||||
Arc::new(Mutex::new(HashMap::new()));
|
||||
|
||||
let (tx, rx) = oneshot::channel();
|
||||
{
|
||||
let mut map = pending.lock().await;
|
||||
map.insert(0, tx);
|
||||
}
|
||||
|
||||
let handle = spawn_jsonrpc_reader(reader, pending.clone(), "test".into());
|
||||
|
||||
let resp = rx.await.expect("should receive the real id=0 response");
|
||||
assert_eq!(resp.id, Some(0));
|
||||
assert!(resp.result.is_some());
|
||||
|
||||
handle.await.expect("reader task should finish");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,7 +15,7 @@ use tokio::sync::{Mutex, oneshot};
|
||||
use tokio::task::JoinHandle;
|
||||
|
||||
use crate::tools::mcp::protocol::{McpRequest, McpResponse};
|
||||
use crate::tools::mcp::transport::{McpTransport, spawn_jsonrpc_reader, stream_transport_send};
|
||||
use crate::tools::mcp::transport::{McpTransport, spawn_jsonrpc_reader, write_jsonrpc_line};
|
||||
use crate::tools::tool::ToolError;
|
||||
|
||||
/// MCP transport that communicates over a Unix domain socket.
|
||||
@@ -91,14 +91,63 @@ impl McpTransport for UnixMcpTransport {
|
||||
request: &McpRequest,
|
||||
_headers: &HashMap<String, String>,
|
||||
) -> Result<McpResponse, ToolError> {
|
||||
stream_transport_send(
|
||||
&self.writer,
|
||||
&self.pending,
|
||||
request,
|
||||
&self.server_name,
|
||||
Duration::from_secs(30),
|
||||
)
|
||||
.await
|
||||
// JSON-RPC notifications (no id) are fire-and-forget: the server
|
||||
// will not send a response, so we must not wait for one.
|
||||
if request.id.is_none() {
|
||||
let mut writer = self.writer.lock().await;
|
||||
write_jsonrpc_line(&mut *writer, request).await?;
|
||||
return Ok(McpResponse {
|
||||
jsonrpc: "2.0".to_string(),
|
||||
id: None,
|
||||
result: None,
|
||||
error: None,
|
||||
});
|
||||
}
|
||||
|
||||
let id = request.id.unwrap_or(0);
|
||||
let (tx, rx) = oneshot::channel();
|
||||
|
||||
// Register the pending response handler before writing the request,
|
||||
// so we don't miss a fast response from the server.
|
||||
{
|
||||
let mut pending = self.pending.lock().await;
|
||||
pending.insert(id, tx);
|
||||
}
|
||||
|
||||
// Write the request to the socket.
|
||||
{
|
||||
let mut writer = self.writer.lock().await;
|
||||
if let Err(e) = write_jsonrpc_line(&mut *writer, request).await {
|
||||
// Remove the pending entry on write failure.
|
||||
let mut pending = self.pending.lock().await;
|
||||
pending.remove(&id);
|
||||
return Err(e);
|
||||
}
|
||||
}
|
||||
|
||||
// Wait for the response with a timeout.
|
||||
let timeout = Duration::from_secs(30);
|
||||
match tokio::time::timeout(timeout, rx).await {
|
||||
Ok(Ok(response)) => Ok(response),
|
||||
Ok(Err(_)) => {
|
||||
// Sender was dropped (reader task ended). Clean up pending entry.
|
||||
let mut pending = self.pending.lock().await;
|
||||
pending.remove(&id);
|
||||
Err(ToolError::ExternalService(format!(
|
||||
"[{}] MCP server closed connection before responding to request {:?}",
|
||||
self.server_name, request.id
|
||||
)))
|
||||
}
|
||||
Err(_) => {
|
||||
// Timeout: remove the pending entry.
|
||||
let mut pending = self.pending.lock().await;
|
||||
pending.remove(&id);
|
||||
Err(ToolError::ExternalService(format!(
|
||||
"[{}] Timeout waiting for response to request {:?} after {:?}",
|
||||
self.server_name, request.id, timeout
|
||||
)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn shutdown(&self) -> Result<(), ToolError> {
|
||||
|
||||
@@ -558,7 +558,48 @@ mod tests {
|
||||
// Routine tools
|
||||
(
|
||||
"routine_create",
|
||||
crate::tools::builtin::routine::routine_create_parameters_schema(),
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": { "type": "string", "description": "Routine name" },
|
||||
"description": { "type": "string", "description": "What it does" },
|
||||
"trigger_type": {
|
||||
"type": "string",
|
||||
"enum": ["cron", "event", "system_event", "manual"],
|
||||
"description": "When the routine fires"
|
||||
},
|
||||
"schedule": { "type": "string", "description": "Cron expression" },
|
||||
"event_pattern": { "type": "string", "description": "Regex pattern" },
|
||||
"event_channel": { "type": "string", "description": "Channel filter" },
|
||||
"event_source": { "type": "string", "description": "System event source" },
|
||||
"event_type": { "type": "string", "description": "System event type" },
|
||||
"event_filters": {
|
||||
"type": "object",
|
||||
"additionalProperties": { "type": "string" },
|
||||
"description": "Exact-match payload filters"
|
||||
},
|
||||
"prompt": { "type": "string", "description": "Instructions" },
|
||||
"context_paths": {
|
||||
"type": "array",
|
||||
"items": { "type": "string" },
|
||||
"description": "Workspace paths to load"
|
||||
},
|
||||
"action_type": {
|
||||
"type": "string",
|
||||
"enum": ["lightweight", "full_job"],
|
||||
"description": "Execution mode"
|
||||
},
|
||||
"cooldown_secs": { "type": "integer", "description": "Min seconds between fires" },
|
||||
"tool_permissions": {
|
||||
"type": "array",
|
||||
"items": { "type": "string" },
|
||||
"description": "Pre-authorized tools for full_job mode"
|
||||
},
|
||||
"notify_channel": { "type": "string", "description": "Channel for message tool" },
|
||||
"notify_user": { "type": "string", "description": "User/target to notify" }
|
||||
},
|
||||
"required": ["name", "trigger_type", "prompt"]
|
||||
}),
|
||||
),
|
||||
(
|
||||
"routine_list",
|
||||
@@ -570,7 +611,17 @@ mod tests {
|
||||
),
|
||||
(
|
||||
"routine_update",
|
||||
crate::tools::builtin::routine::routine_update_parameters_schema(),
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": { "type": "string", "description": "Name" },
|
||||
"enabled": { "type": "boolean", "description": "Toggle" },
|
||||
"prompt": { "type": "string", "description": "New prompt" },
|
||||
"schedule": { "type": "string", "description": "New cron schedule" },
|
||||
"description": { "type": "string", "description": "New description" }
|
||||
},
|
||||
"required": ["name"]
|
||||
}),
|
||||
),
|
||||
(
|
||||
"routine_delete",
|
||||
|
||||
+3
-48
@@ -430,24 +430,9 @@ pub fn redact_params(params: &serde_json::Value, sensitive: &[&str]) -> serde_js
|
||||
/// Properties without a `"type"` field are allowed (freeform/any-type).
|
||||
/// This is an intentional pattern used by tools like `json` and `http` for
|
||||
/// OpenAI compatibility, since union types with arrays require `items`.
|
||||
/// Maximum nesting depth for tool schema validation to prevent stack overflow
|
||||
/// on maliciously crafted schemas.
|
||||
const MAX_SCHEMA_DEPTH: usize = 16;
|
||||
|
||||
pub fn validate_tool_schema(schema: &serde_json::Value, path: &str) -> Vec<String> {
|
||||
validate_tool_schema_inner(schema, path, 0)
|
||||
}
|
||||
|
||||
fn validate_tool_schema_inner(schema: &serde_json::Value, path: &str, depth: usize) -> Vec<String> {
|
||||
let mut errors = Vec::new();
|
||||
|
||||
if depth > MAX_SCHEMA_DEPTH {
|
||||
errors.push(format!(
|
||||
"{path}: schema nesting exceeds maximum depth of {MAX_SCHEMA_DEPTH}"
|
||||
));
|
||||
return errors;
|
||||
}
|
||||
|
||||
// Rule 1: must have "type": "object" at this level
|
||||
match schema.get("type").and_then(|t| t.as_str()) {
|
||||
Some("object") => {}
|
||||
@@ -489,17 +474,14 @@ fn validate_tool_schema_inner(schema: &serde_json::Value, path: &str, depth: usi
|
||||
if let Some(prop_type) = prop.get("type").and_then(|t| t.as_str()) {
|
||||
match prop_type {
|
||||
"object" => {
|
||||
errors.extend(validate_tool_schema_inner(prop, &prop_path, depth + 1));
|
||||
errors.extend(validate_tool_schema(prop, &prop_path));
|
||||
}
|
||||
"array" => {
|
||||
if let Some(items) = prop.get("items") {
|
||||
// If items is an object type, recurse
|
||||
if items.get("type").and_then(|t| t.as_str()) == Some("object") {
|
||||
errors.extend(validate_tool_schema_inner(
|
||||
items,
|
||||
&format!("{prop_path}.items"),
|
||||
depth + 1,
|
||||
));
|
||||
errors
|
||||
.extend(validate_tool_schema(items, &format!("{prop_path}.items")));
|
||||
}
|
||||
} else {
|
||||
errors.push(format!("{prop_path}: array property missing \"items\""));
|
||||
@@ -828,33 +810,6 @@ mod tests {
|
||||
assert!(errors[0].contains("\"missing_field\""));
|
||||
}
|
||||
|
||||
/// Regression test for issue #975: deeply nested schemas must not cause
|
||||
/// stack overflow. The validator should stop at MAX_SCHEMA_DEPTH and
|
||||
/// report an error instead of recursing infinitely.
|
||||
#[test]
|
||||
fn test_validate_schema_depth_limit() {
|
||||
// Build a schema nested 20 levels deep (exceeds MAX_SCHEMA_DEPTH=16)
|
||||
let mut schema = serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"leaf": { "type": "string" }
|
||||
}
|
||||
});
|
||||
for _ in 0..20 {
|
||||
schema = serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"nested": schema
|
||||
}
|
||||
});
|
||||
}
|
||||
let errors = validate_tool_schema(&schema, "test");
|
||||
assert!(
|
||||
errors.iter().any(|e| e.contains("maximum depth")),
|
||||
"expected depth limit error, got: {errors:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_approval_context_autonomous_allows_unless_auto_approved() {
|
||||
let ctx = ApprovalContext::autonomous();
|
||||
|
||||
@@ -101,75 +101,24 @@ pub struct CapabilitiesFile {
|
||||
pub capabilities: Option<Box<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> {
|
||||
let mut caps = serde_json::from_str::<Self>(json).map(Self::resolve_nested)?;
|
||||
caps.enforce_limits();
|
||||
Ok(caps)
|
||||
serde_json::from_str::<Self>(json).map(Self::resolve_nested)
|
||||
}
|
||||
|
||||
/// Parse from JSON bytes.
|
||||
pub fn from_bytes(bytes: &[u8]) -> Result<Self, serde_json::Error> {
|
||||
let mut caps = serde_json::from_slice::<Self>(bytes).map(Self::resolve_nested)?;
|
||||
caps.enforce_limits();
|
||||
Ok(caps)
|
||||
}
|
||||
|
||||
/// Truncate oversized fields to prevent unbounded memory usage.
|
||||
fn enforce_limits(&mut self) {
|
||||
// Truncate oversized description (issue #976)
|
||||
if let Some(ref desc) = self.description
|
||||
&& desc.len() > MAX_DESCRIPTION_CHARS
|
||||
{
|
||||
let truncated = &desc[..desc.floor_char_boundary(MAX_DESCRIPTION_CHARS)];
|
||||
tracing::warn!(
|
||||
"Capabilities description truncated from {} to {} chars",
|
||||
desc.len(),
|
||||
MAX_DESCRIPTION_CHARS,
|
||||
);
|
||||
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;
|
||||
}
|
||||
}
|
||||
serde_json::from_slice::<Self>(bytes).map(Self::resolve_nested)
|
||||
}
|
||||
|
||||
/// Merge nested `capabilities` wrapper into top-level fields.
|
||||
///
|
||||
/// Channel-level JSON nests tool capabilities under `"capabilities"`.
|
||||
/// This promotes the inner fields so callers can access them uniformly.
|
||||
/// Maximum nesting depth for capabilities resolution.
|
||||
const MAX_NESTED_DEPTH: usize = 8;
|
||||
|
||||
fn resolve_nested(self) -> Self {
|
||||
self.resolve_nested_inner(0)
|
||||
}
|
||||
|
||||
fn resolve_nested_inner(mut self, depth: usize) -> Self {
|
||||
if depth > Self::MAX_NESTED_DEPTH {
|
||||
tracing::warn!(
|
||||
"Capabilities nesting exceeds maximum depth of {}, stopping resolution",
|
||||
Self::MAX_NESTED_DEPTH
|
||||
);
|
||||
return self;
|
||||
}
|
||||
fn resolve_nested(mut self) -> Self {
|
||||
if let Some(inner) = self.capabilities.take() {
|
||||
let inner = inner.resolve_nested_inner(depth + 1);
|
||||
let inner = inner.resolve_nested();
|
||||
self.description = self.description.or(inner.description);
|
||||
self.parameters = self.parameters.or(inner.parameters);
|
||||
self.http = self.http.or(inner.http);
|
||||
@@ -1434,63 +1383,4 @@ mod tests {
|
||||
"Outer description should take precedence over inner"
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression test for issue #974: deeply nested capabilities wrappers
|
||||
/// must not cause stack overflow. resolve_nested should stop at
|
||||
/// MAX_NESTED_DEPTH and return gracefully.
|
||||
#[test]
|
||||
fn test_resolve_nested_depth_limit() {
|
||||
// Build a capabilities file nested beyond MAX_NESTED_DEPTH (8).
|
||||
// The description is at the innermost level which is beyond the limit,
|
||||
// so it won't be resolved — the key assertion is no stack overflow.
|
||||
let mut json = r#"{ "description": "leaf" }"#.to_string();
|
||||
for _ in 0..20 {
|
||||
json = format!(r#"{{ "capabilities": {json} }}"#);
|
||||
}
|
||||
// Should not stack overflow — this is the primary assertion.
|
||||
let _caps = CapabilitiesFile::from_json(&json).unwrap();
|
||||
}
|
||||
|
||||
/// Regression test for issue #976: oversized description strings are truncated.
|
||||
#[test]
|
||||
fn test_description_truncated_at_limit() {
|
||||
let long_desc = "x".repeat(10_000);
|
||||
let json = format!(r#"{{ "description": "{long_desc}" }}"#);
|
||||
let caps = CapabilitiesFile::from_json(&json).unwrap();
|
||||
let desc = caps.description.unwrap();
|
||||
assert!(
|
||||
desc.len() <= super::MAX_DESCRIPTION_CHARS + 50, // allow for minor overhead
|
||||
"description should be truncated to ~{} chars, got {}",
|
||||
super::MAX_DESCRIPTION_CHARS,
|
||||
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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
+5
-197
@@ -1104,18 +1104,7 @@ async fn resolve_host_credentials(
|
||||
) -> Vec<ResolvedHostCredential> {
|
||||
let store = match store {
|
||||
Some(s) => s,
|
||||
None => {
|
||||
// If tool requires credentials but has no secrets store, this is a configuration error
|
||||
if let Some(http_cap) = &capabilities.http
|
||||
&& !http_cap.credentials.is_empty()
|
||||
{
|
||||
tracing::warn!(
|
||||
user_id = %user_id,
|
||||
"WASM tool requires credentials but secrets_store is not configured - authentication will fail"
|
||||
);
|
||||
}
|
||||
return Vec::new();
|
||||
}
|
||||
None => return Vec::new(),
|
||||
};
|
||||
|
||||
// Check if the access token needs refreshing before resolving credentials.
|
||||
@@ -1166,37 +1155,13 @@ async fn resolve_host_credentials(
|
||||
continue;
|
||||
}
|
||||
|
||||
// Try to get credential under the provided user_id first.
|
||||
// If not found and user_id != "default", fallback to "default" (global credentials).
|
||||
// This handles OAuth tokens stored globally under "default" but accessed from routine contexts.
|
||||
let secret = match store.get_decrypted(user_id, &mapping.secret_name).await {
|
||||
Ok(s) => Some(s),
|
||||
Ok(s) => s,
|
||||
Err(e) => {
|
||||
// If lookup fails and we're not already looking up "default", try "default" as fallback
|
||||
if user_id != "default" {
|
||||
tracing::debug!(
|
||||
secret_name = %mapping.secret_name,
|
||||
user_id = %user_id,
|
||||
error = %e,
|
||||
"Credential not found for user, trying default global credentials"
|
||||
);
|
||||
store
|
||||
.get_decrypted("default", &mapping.secret_name)
|
||||
.await
|
||||
.ok()
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
let secret = match secret {
|
||||
Some(s) => s,
|
||||
None => {
|
||||
tracing::warn!(
|
||||
tracing::debug!(
|
||||
secret_name = %mapping.secret_name,
|
||||
user_id = %user_id,
|
||||
"Could not resolve credential for WASM tool (not found in user context or default)"
|
||||
error = %e,
|
||||
"Could not resolve credential for WASM tool (auth may not be configured)"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
@@ -2093,161 +2058,4 @@ mod tests {
|
||||
"Leak scan on post-injection headers should block the Slack token"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_resolve_host_credentials_fallback_to_default_user() {
|
||||
use crate::secrets::{CredentialLocation, CredentialMapping, SecretsStore};
|
||||
use crate::tools::wasm::capabilities::HttpCapability;
|
||||
use crate::tools::wasm::wrapper::resolve_host_credentials;
|
||||
|
||||
let store = test_secrets_store();
|
||||
|
||||
// Store a token under the "default" global user
|
||||
store
|
||||
.create(
|
||||
"default",
|
||||
crate::secrets::CreateSecretParams::new("google_oauth_token", "global_token_value"),
|
||||
)
|
||||
.await
|
||||
.expect("Failed to store global token"); // safety: test code only
|
||||
|
||||
// Create capabilities requiring this credential
|
||||
let mut creds = std::collections::HashMap::new();
|
||||
creds.insert(
|
||||
"google_oauth_token".to_string(),
|
||||
CredentialMapping {
|
||||
secret_name: "google_oauth_token".to_string(),
|
||||
location: CredentialLocation::AuthorizationBearer,
|
||||
host_patterns: vec!["sheets.googleapis.com".to_string()],
|
||||
},
|
||||
);
|
||||
let caps = Capabilities {
|
||||
http: Some(HttpCapability {
|
||||
allowlist: vec![],
|
||||
credentials: creds,
|
||||
rate_limit: crate::tools::wasm::capabilities::RateLimitConfig::default(),
|
||||
max_request_bytes: 1024 * 1024,
|
||||
max_response_bytes: 10 * 1024 * 1024,
|
||||
timeout: std::time::Duration::from_secs(30),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
// Resolve credentials for a different user (routine context)
|
||||
// Should fallback to "default" and find the token
|
||||
let result = resolve_host_credentials(&caps, Some(&store), "routine_user_123", None).await;
|
||||
|
||||
assert!(!result.is_empty(), "fallback to default"); // safety: test code only
|
||||
assert_eq!(result[0].secret_value, "global_token_value"); // safety: test code only
|
||||
}
|
||||
|
||||
fn test_capabilities_with_google_oauth() -> Capabilities {
|
||||
use crate::secrets::{CredentialLocation, CredentialMapping};
|
||||
use crate::tools::wasm::capabilities::HttpCapability;
|
||||
|
||||
let mut creds = std::collections::HashMap::new();
|
||||
creds.insert(
|
||||
"google_oauth_token".to_string(),
|
||||
CredentialMapping {
|
||||
secret_name: "google_oauth_token".to_string(),
|
||||
location: CredentialLocation::AuthorizationBearer,
|
||||
host_patterns: vec!["sheets.googleapis.com".to_string()],
|
||||
},
|
||||
);
|
||||
Capabilities {
|
||||
http: Some(HttpCapability {
|
||||
allowlist: vec![],
|
||||
credentials: creds,
|
||||
rate_limit: crate::tools::wasm::capabilities::RateLimitConfig::default(),
|
||||
max_request_bytes: 1024 * 1024,
|
||||
max_response_bytes: 10 * 1024 * 1024,
|
||||
timeout: std::time::Duration::from_secs(30),
|
||||
}),
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_resolve_host_credentials_prefers_user_specific_over_default() {
|
||||
use crate::secrets::SecretsStore;
|
||||
use crate::tools::wasm::wrapper::resolve_host_credentials;
|
||||
|
||||
let store = test_secrets_store();
|
||||
|
||||
// Store token under "default" (global)
|
||||
store
|
||||
.create(
|
||||
"default",
|
||||
crate::secrets::CreateSecretParams::new("google_oauth_token", "global_token"),
|
||||
)
|
||||
.await
|
||||
.expect("Failed to store global token"); // safety: test code only
|
||||
|
||||
// Store token under user_123 (user-specific)
|
||||
store
|
||||
.create(
|
||||
"user_123",
|
||||
crate::secrets::CreateSecretParams::new(
|
||||
"google_oauth_token",
|
||||
"user_specific_token",
|
||||
),
|
||||
)
|
||||
.await
|
||||
.expect("Failed to store user token"); // safety: test code only
|
||||
|
||||
// Create capabilities
|
||||
let caps = test_capabilities_with_google_oauth();
|
||||
|
||||
// Resolve credentials for user_123
|
||||
// Should prefer user_123's token over default
|
||||
let result = resolve_host_credentials(&caps, Some(&store), "user_123", None).await;
|
||||
|
||||
assert!(!result.is_empty(), "has user credentials"); // safety: test code only
|
||||
assert_eq!(result[0].secret_value, "user_specific_token", "user token"); // safety: test code only
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_resolve_host_credentials_no_fallback_when_already_default() {
|
||||
use crate::secrets::SecretsStore;
|
||||
use crate::tools::wasm::wrapper::resolve_host_credentials;
|
||||
|
||||
let store = test_secrets_store();
|
||||
|
||||
// Only store token under "default" (not a duplicate)
|
||||
store
|
||||
.create(
|
||||
"default",
|
||||
crate::secrets::CreateSecretParams::new("google_oauth_token", "default_token"),
|
||||
)
|
||||
.await
|
||||
.expect("Failed to store default token"); // safety: test code only
|
||||
|
||||
// Create capabilities
|
||||
let caps = test_capabilities_with_google_oauth();
|
||||
|
||||
// Resolve credentials for "default" user
|
||||
// Should NOT attempt fallback (already looking up default)
|
||||
let result = resolve_host_credentials(&caps, Some(&store), "default", None).await;
|
||||
|
||||
assert!(!result.is_empty(), "Should find default token"); // safety: test code only
|
||||
assert_eq!(result[0].secret_value, "default_token"); // safety: test code only
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_resolve_host_credentials_missing_secret_warns() {
|
||||
use crate::tools::wasm::wrapper::resolve_host_credentials;
|
||||
|
||||
let store = test_secrets_store();
|
||||
|
||||
// Don't store any token
|
||||
|
||||
// Create capabilities expecting a credential
|
||||
let caps = test_capabilities_with_google_oauth();
|
||||
|
||||
// Resolve credentials when neither user nor default has the token
|
||||
let result = resolve_host_credentials(&caps, Some(&store), "user_456", None).await;
|
||||
|
||||
// Should return empty since credential can't be found anywhere
|
||||
assert!(result.is_empty(), "no credentials found"); // safety: test code only
|
||||
}
|
||||
}
|
||||
|
||||
+4
-4
@@ -1108,10 +1108,9 @@ impl<'a> LoopDelegate for JobDelegate<'a> {
|
||||
return LoopSignal::InjectMessage(content);
|
||||
}
|
||||
|
||||
// Check for terminal or post-completion state. The loop should stop when the
|
||||
// job has been cancelled, failed, or already completed — but NOT when Stuck,
|
||||
// because Stuck is recoverable (Stuck -> InProgress via self-repair).
|
||||
// Stopping on Stuck would prevent recovery from resuming the worker (issue #892).
|
||||
// Check for terminal or non-progressing state. The loop should stop when the
|
||||
// job has been cancelled, failed, stuck, or already completed — not just the
|
||||
// three states that `is_terminal()` covers (Accepted/Failed/Cancelled).
|
||||
if let Ok(ctx) = self
|
||||
.worker
|
||||
.context_manager()
|
||||
@@ -1121,6 +1120,7 @@ impl<'a> LoopDelegate for JobDelegate<'a> {
|
||||
ctx.state,
|
||||
JobState::Cancelled
|
||||
| JobState::Failed
|
||||
| JobState::Stuck
|
||||
| JobState::Completed
|
||||
| JobState::Submitted
|
||||
| JobState::Accepted
|
||||
|
||||
+1
-92
@@ -160,7 +160,7 @@ async def ironclaw_server(ironclaw_binary, mock_llm_server, wasm_tools_dir):
|
||||
"LIBSQL_PATH": os.path.join(_DB_TMPDIR.name, "e2e.db"),
|
||||
"SANDBOX_ENABLED": "false",
|
||||
"SKILLS_ENABLED": "true",
|
||||
"ROUTINES_ENABLED": "true",
|
||||
"ROUTINES_ENABLED": "false",
|
||||
"HEARTBEAT_ENABLED": "false",
|
||||
"EMBEDDING_ENABLED": "false",
|
||||
# WASM tool/channel support
|
||||
@@ -220,97 +220,6 @@ async def ironclaw_server(ironclaw_binary, mock_llm_server, wasm_tools_dir):
|
||||
proc.kill()
|
||||
|
||||
|
||||
@pytest.fixture(scope="session")
|
||||
async def ironclaw_server_with_webhook_secret(ironclaw_binary, mock_llm_server, wasm_tools_dir):
|
||||
"""Start ironclaw with HTTP_WEBHOOK_SECRET configured for webhook tests.
|
||||
|
||||
Yields a dict with:
|
||||
- 'url': base URL of the gateway
|
||||
- 'secret': the webhook secret value
|
||||
"""
|
||||
gateway_port = _find_free_port()
|
||||
webhook_secret = "test-webhook-secret-e2e-12345"
|
||||
env = {
|
||||
# Minimal env: PATH for process spawning, HOME for Rust/cargo defaults
|
||||
"PATH": os.environ.get("PATH", "/usr/bin:/bin"),
|
||||
"HOME": os.environ.get("HOME", "/tmp"),
|
||||
"RUST_LOG": "ironclaw=info",
|
||||
"RUST_BACKTRACE": "1",
|
||||
"GATEWAY_ENABLED": "true",
|
||||
"GATEWAY_HOST": "127.0.0.1",
|
||||
"GATEWAY_PORT": str(gateway_port),
|
||||
"GATEWAY_AUTH_TOKEN": AUTH_TOKEN,
|
||||
"GATEWAY_USER_ID": "e2e-tester",
|
||||
"HTTP_WEBHOOK_SECRET": webhook_secret,
|
||||
"CLI_ENABLED": "false",
|
||||
"LLM_BACKEND": "openai_compatible",
|
||||
"LLM_BASE_URL": mock_llm_server,
|
||||
"LLM_MODEL": "mock-model",
|
||||
"DATABASE_BACKEND": "libsql",
|
||||
"LIBSQL_PATH": os.path.join(_DB_TMPDIR.name, "e2e-webhook.db"),
|
||||
"SANDBOX_ENABLED": "false",
|
||||
"SKILLS_ENABLED": "true",
|
||||
"ROUTINES_ENABLED": "false",
|
||||
"HEARTBEAT_ENABLED": "false",
|
||||
"EMBEDDING_ENABLED": "false",
|
||||
# WASM tool/channel support
|
||||
"WASM_ENABLED": "true",
|
||||
"WASM_TOOLS_DIR": wasm_tools_dir,
|
||||
"WASM_CHANNELS_DIR": _WASM_CHANNELS_TMPDIR.name,
|
||||
# Prevent onboarding wizard from triggering
|
||||
"ONBOARD_COMPLETED": "true",
|
||||
# Force gateway OAuth callback mode (non-loopback URL) and point
|
||||
# token exchange at mock_llm.py so OAuth tests work without Google.
|
||||
"IRONCLAW_OAUTH_CALLBACK_URL": "https://oauth.test.example/oauth/callback",
|
||||
"IRONCLAW_OAUTH_EXCHANGE_URL": mock_llm_server,
|
||||
}
|
||||
# Forward LLVM coverage instrumentation env vars when present
|
||||
COV_ENV_PREFIXES = ("CARGO_LLVM_COV", "LLVM_")
|
||||
COV_ENV_EXTRAS = ("CARGO_ENCODED_RUSTFLAGS", "CARGO_INCREMENTAL")
|
||||
for key, val in os.environ.items():
|
||||
if key.startswith(COV_ENV_PREFIXES) or key in COV_ENV_EXTRAS:
|
||||
env[key] = val
|
||||
proc = await asyncio.create_subprocess_exec(
|
||||
ironclaw_binary, "--no-onboard",
|
||||
stdin=asyncio.subprocess.DEVNULL,
|
||||
stdout=asyncio.subprocess.PIPE,
|
||||
stderr=asyncio.subprocess.PIPE,
|
||||
env=env,
|
||||
)
|
||||
base_url = f"http://127.0.0.1:{gateway_port}"
|
||||
try:
|
||||
await wait_for_ready(f"{base_url}/api/health", timeout=60)
|
||||
yield {
|
||||
"url": base_url,
|
||||
"secret": webhook_secret,
|
||||
}
|
||||
except TimeoutError:
|
||||
# Dump stderr so CI logs show why the server failed to start
|
||||
returncode = proc.returncode
|
||||
stderr_bytes = b""
|
||||
if proc.stderr:
|
||||
try:
|
||||
stderr_bytes = await asyncio.wait_for(proc.stderr.read(8192), timeout=2)
|
||||
except (asyncio.TimeoutError, Exception):
|
||||
pass
|
||||
stderr_text = stderr_bytes.decode("utf-8", errors="replace")
|
||||
proc.kill()
|
||||
pytest.fail(
|
||||
f"ironclaw server with webhook secret failed to start on port {gateway_port} "
|
||||
f"(returncode={returncode}).\nstderr:\n{stderr_text}"
|
||||
)
|
||||
finally:
|
||||
if proc.returncode is None:
|
||||
# Use SIGINT (not SIGTERM) so tokio's ctrl_c handler triggers a
|
||||
# graceful shutdown. This lets the LLVM coverage runtime run its
|
||||
# atexit handler and flush .profraw files for cargo-llvm-cov.
|
||||
proc.send_signal(signal.SIGINT)
|
||||
try:
|
||||
await asyncio.wait_for(proc.wait(), timeout=10)
|
||||
except asyncio.TimeoutError:
|
||||
proc.kill()
|
||||
|
||||
|
||||
@pytest.fixture(scope="session")
|
||||
async def browser(ironclaw_server):
|
||||
"""Session-scoped Playwright browser instance.
|
||||
|
||||
@@ -1,13 +0,0 @@
|
||||
Metadata-Version: 2.4
|
||||
Name: ironclaw-e2e
|
||||
Version: 0.1.0
|
||||
Requires-Python: >=3.11
|
||||
Requires-Dist: pytest>=8.0
|
||||
Requires-Dist: pytest-asyncio>=0.23
|
||||
Requires-Dist: pytest-playwright>=0.5
|
||||
Requires-Dist: pytest-timeout>=2.3
|
||||
Requires-Dist: playwright>=1.40
|
||||
Requires-Dist: aiohttp>=3.9
|
||||
Requires-Dist: httpx>=0.27
|
||||
Provides-Extra: vision
|
||||
Requires-Dist: anthropic>=0.40; extra == "vision"
|
||||
@@ -1,22 +0,0 @@
|
||||
README.md
|
||||
pyproject.toml
|
||||
ironclaw_e2e.egg-info/PKG-INFO
|
||||
ironclaw_e2e.egg-info/SOURCES.txt
|
||||
ironclaw_e2e.egg-info/dependency_links.txt
|
||||
ironclaw_e2e.egg-info/requires.txt
|
||||
ironclaw_e2e.egg-info/top_level.txt
|
||||
scenarios/__init__.py
|
||||
scenarios/test_chat.py
|
||||
scenarios/test_connection.py
|
||||
scenarios/test_csp.py
|
||||
scenarios/test_extension_oauth.py
|
||||
scenarios/test_extensions.py
|
||||
scenarios/test_html_injection.py
|
||||
scenarios/test_oauth_credential_fallback.py
|
||||
scenarios/test_pairing.py
|
||||
scenarios/test_routine_oauth_credential_injection.py
|
||||
scenarios/test_skills.py
|
||||
scenarios/test_sse_reconnect.py
|
||||
scenarios/test_tool_approval.py
|
||||
scenarios/test_tool_execution.py
|
||||
scenarios/test_wasm_lifecycle.py
|
||||
@@ -1 +0,0 @@
|
||||
|
||||
@@ -1,10 +0,0 @@
|
||||
pytest>=8.0
|
||||
pytest-asyncio>=0.23
|
||||
pytest-playwright>=0.5
|
||||
pytest-timeout>=2.3
|
||||
playwright>=1.40
|
||||
aiohttp>=3.9
|
||||
httpx>=0.27
|
||||
|
||||
[vision]
|
||||
anthropic>=0.40
|
||||
@@ -1 +0,0 @@
|
||||
scenarios
|
||||
@@ -1,110 +0,0 @@
|
||||
"""OAuth credential fallback e2e tests.
|
||||
|
||||
Tests that OAuth tokens stored globally under 'default' user are properly
|
||||
injected when WASM tools make HTTP requests. This validates the fix for:
|
||||
https://github.com/nearai/ironclaw/issues/999
|
||||
|
||||
Note: Full routine execution testing is limited because routines are disabled
|
||||
in the e2e test environment (ROUTINES_ENABLED=false in conftest.py). This test
|
||||
validates the OAuth + credential injection flow at the REST API level.
|
||||
|
||||
Unit tests in src/tools/wasm/wrapper.rs provide additional coverage of the
|
||||
fallback mechanism itself.
|
||||
"""
|
||||
|
||||
from helpers import api_post, api_get
|
||||
import pytest
|
||||
|
||||
|
||||
async def test_oauth_credential_injection_after_gmail_auth(ironclaw_server):
|
||||
"""Verify that after OAuth, tool HTTP requests include credentials.
|
||||
|
||||
This is an indirect test: we verify that gmail shows as authenticated
|
||||
and that its tools are registered. A full e2e test would require:
|
||||
1. Enabling ROUTINES_ENABLED=true in conftest.py
|
||||
2. Creating a routine that calls a WASM tool with OAuth
|
||||
3. Triggering the routine and verifying the request succeeded
|
||||
|
||||
The unit tests in src/tools/wasm/wrapper.rs validate the credential
|
||||
fallback mechanism (trying 'default' user when user-specific lookup fails).
|
||||
"""
|
||||
|
||||
# First, ensure gmail is installed and authenticated
|
||||
# (Reuse from test_extension_oauth.py if running in sequence)
|
||||
r = await api_get(ironclaw_server, "/api/extensions")
|
||||
extensions = r.json().get("extensions", [])
|
||||
gmail = next((ext for ext in extensions if ext["name"] == "gmail"), None)
|
||||
|
||||
if gmail is None:
|
||||
# Install gmail
|
||||
r = await api_post(
|
||||
ironclaw_server,
|
||||
"/api/extensions/install",
|
||||
json={"name": "gmail"},
|
||||
timeout=180,
|
||||
)
|
||||
assert r.status_code == 200, f"Failed to install gmail: {r.text}"
|
||||
|
||||
# Verify gmail is authenticated (it should be if oauth flow completed)
|
||||
r = await api_get(ironclaw_server, "/api/extensions")
|
||||
extensions = r.json().get("extensions", [])
|
||||
gmail = next((ext for ext in extensions if ext["name"] == "gmail"), None)
|
||||
assert gmail is not None, "gmail not found in extensions"
|
||||
|
||||
# Authenticated tools should have credentials available for injection
|
||||
if gmail.get("authenticated"):
|
||||
tools = gmail.get("tools", [])
|
||||
assert (
|
||||
len(tools) > 0
|
||||
), f"Authenticated gmail should have tools registered: {gmail}"
|
||||
|
||||
# Tools should be callable (which requires credential injection)
|
||||
# In a full e2e with routines enabled, we would:
|
||||
# 1. Call a gmail tool from a routine
|
||||
# 2. Verify the HTTP request included the OAuth token
|
||||
# 3. Verify no 403 "unregistered callers" error
|
||||
|
||||
|
||||
async def test_tool_registry_lists_authenticated_extensions(ironclaw_server):
|
||||
"""Verify authenticated extensions' tools are registered in tool registry.
|
||||
|
||||
Tools from authenticated extensions should have credentials pre-injected
|
||||
before HTTP requests are made. This validates the end of the injection
|
||||
pipeline (credential resolution -> WASM execution -> HTTP request).
|
||||
"""
|
||||
|
||||
# Get extensions list
|
||||
r = await api_get(ironclaw_server, "/api/extensions")
|
||||
extensions = r.json().get("extensions", [])
|
||||
|
||||
# Authenticated extensions should appear
|
||||
authenticated = [ext for ext in extensions if ext.get("authenticated")]
|
||||
|
||||
# At minimum, verify the endpoint works and structure is correct
|
||||
for ext in authenticated:
|
||||
assert "name" in ext
|
||||
assert "tools" in ext
|
||||
assert isinstance(ext["tools"], list)
|
||||
|
||||
|
||||
async def test_credential_fallback_documented_in_code(ironclaw_server):
|
||||
"""Verify the credential fallback fix is present.
|
||||
|
||||
This is a documentation test that the bug fix for issue #999 is
|
||||
actually in the code. The real validation happens in unit tests:
|
||||
- test_resolve_host_credentials_fallback_to_default_user
|
||||
- test_resolve_host_credentials_prefers_user_specific_over_default
|
||||
- test_resolve_host_credentials_no_fallback_when_already_default
|
||||
|
||||
If these unit tests pass, the fix is working correctly.
|
||||
"""
|
||||
|
||||
# This test serves as a reminder that:
|
||||
# 1. OAuth tokens are stored globally under user_id="default"
|
||||
# 2. When routines execute, they use routine.user_id (not "default")
|
||||
# 3. The fix adds credential fallback: try user_id first, then "default"
|
||||
# 4. This allows global OAuth tokens to be used in routine contexts
|
||||
|
||||
# No specific assertion needed — presence of this test file documents
|
||||
# the fix. Actual validation is in unit tests.
|
||||
assert True
|
||||
@@ -1,182 +0,0 @@
|
||||
"""Playwright e2e tests for OAuth credential injection in routines.
|
||||
|
||||
Tests the full flow for issue #999:
|
||||
1. Complete OAuth for a WASM tool (gmail)
|
||||
2. Create a routine that calls that tool
|
||||
3. Manually trigger the routine
|
||||
4. Verify the tool executes with proper credential injection (no 403 errors)
|
||||
|
||||
This tests that OAuth tokens stored globally under 'default' user are properly
|
||||
accessible in routine execution contexts.
|
||||
"""
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from helpers import SEL, api_post, api_get
|
||||
|
||||
|
||||
async def test_routine_with_oauth_credentials_e2e(page, ironclaw_server):
|
||||
"""Complete flow: OAuth → routine creation → execution → success.
|
||||
|
||||
This is the most comprehensive test for the credential fallback fix.
|
||||
It validates that:
|
||||
1. OAuth tokens are stored globally
|
||||
2. Routines can access those tokens
|
||||
3. WASM tools receive proper Authorization headers
|
||||
4. No 403 "unregistered callers" errors occur
|
||||
"""
|
||||
|
||||
# Step 1: Ensure gmail is installed and authenticated
|
||||
# (Using REST API for setup, consistent with test_extension_oauth.py)
|
||||
r = await api_post(
|
||||
ironclaw_server,
|
||||
"/api/extensions/install",
|
||||
json={"name": "gmail"},
|
||||
timeout=180,
|
||||
)
|
||||
if r.status_code == 200:
|
||||
# Gmail installed successfully
|
||||
pass
|
||||
else:
|
||||
# Might already be installed, that's ok
|
||||
pass
|
||||
|
||||
# Verify gmail is in the extensions list and authenticated
|
||||
r = await api_get(ironclaw_server, "/api/extensions")
|
||||
extensions = r.json().get("extensions", [])
|
||||
gmail = next((ext for ext in extensions if ext["name"] == "gmail"), None)
|
||||
|
||||
if gmail is None:
|
||||
pytest.skip("Gmail extension not available")
|
||||
|
||||
if not gmail.get("authenticated"):
|
||||
pytest.skip("Gmail not authenticated (requires OAuth flow completion)")
|
||||
|
||||
# Step 2: Navigate browser to routines tab and create a routine
|
||||
routines_tab = page.locator('button[data-tab="routines"]')
|
||||
await routines_tab.wait_for(state="visible", timeout=5000)
|
||||
await routines_tab.click()
|
||||
|
||||
# Wait for routines page to load (use load state instead of networkidle to avoid timeout)
|
||||
await page.wait_for_load_state("load", timeout=5000)
|
||||
|
||||
# Look for "Create Routine" or similar button
|
||||
create_btn = page.locator('button:has-text("create"), button:has-text("new")')
|
||||
if await create_btn.count() > 0:
|
||||
await create_btn.first.click()
|
||||
await page.wait_for_load_state("load", timeout=5000)
|
||||
|
||||
# Step 3: Create a routine that calls gmail tool
|
||||
# Fill in routine name
|
||||
name_input = page.locator('input[placeholder*="name"], input[placeholder*="Name"]')
|
||||
if await name_input.count() > 0:
|
||||
await name_input.first.fill("Test OAuth Routine")
|
||||
|
||||
# Fill in routine prompt (should call gmail tool)
|
||||
prompt_input = page.locator('textarea, input[type="text"]:nth-of-type(2)')
|
||||
if await prompt_input.count() > 0:
|
||||
await prompt_input.first.fill(
|
||||
"Check my Gmail inbox and tell me how many unread emails I have."
|
||||
)
|
||||
|
||||
# Look for Save/Create button
|
||||
save_btn = page.locator('button:has-text("save"), button:has-text("create")')
|
||||
if await save_btn.count() > 0:
|
||||
await save_btn.first.click()
|
||||
# Wait for routine to be created
|
||||
await page.wait_for_load_state("networkidle", timeout=5000)
|
||||
|
||||
# Step 4: Trigger the routine manually
|
||||
# Look for a run/execute/trigger button on the routine
|
||||
trigger_btn = page.locator(
|
||||
'button:has-text("run"), button:has-text("trigger"), button:has-text("execute")'
|
||||
)
|
||||
if await trigger_btn.count() > 0:
|
||||
await trigger_btn.first.click()
|
||||
|
||||
# Wait for the routine to execute
|
||||
# In a real scenario, this would make HTTP requests with OAuth credentials
|
||||
await page.wait_for_timeout(3000)
|
||||
|
||||
# Step 5: Verify execution succeeded
|
||||
# Look for success message or check that no error occurred
|
||||
# The key is that if credentials weren't injected, we'd see a 403 error
|
||||
error_msg = page.locator('text="403", text="permission", text="unregistered"')
|
||||
assert (
|
||||
await error_msg.count() == 0
|
||||
), "Should not have permission/403 errors (means credentials weren't injected)"
|
||||
|
||||
# Routine should have output (either success or intelligible failure)
|
||||
output = page.locator(".routine-output, .result, [role=status]")
|
||||
# Just verify the page is responsive and didn't crash
|
||||
assert page.url is not None
|
||||
|
||||
|
||||
async def test_routine_list_shows_oauth_tools_available(page, ironclaw_server):
|
||||
"""Verify routines tab shows that OAuth tools are available for use.
|
||||
|
||||
When a WASM tool is authenticated via OAuth, it should be available
|
||||
for use in routine prompts.
|
||||
"""
|
||||
|
||||
# Navigate to routines tab
|
||||
routines_tab = page.locator('button[data-tab="routines"]')
|
||||
await routines_tab.wait_for(state="visible", timeout=5000)
|
||||
await routines_tab.click()
|
||||
|
||||
await page.wait_for_load_state("load", timeout=5000)
|
||||
|
||||
# If routines are supported, the tab should be visible and functional
|
||||
assert page.url is not None, "Routines tab should be navigable"
|
||||
|
||||
# Check that extensions list shows authenticated tools
|
||||
r = await api_get(ironclaw_server, "/api/extensions")
|
||||
extensions = r.json().get("extensions", [])
|
||||
authenticated = [ext for ext in extensions if ext.get("authenticated")]
|
||||
|
||||
# At minimum, verify that authenticated tools exist
|
||||
# (In a full test, these would be available in the routine editor)
|
||||
if len(authenticated) == 0:
|
||||
pytest.skip("No authenticated extensions available (requires OAuth flow completion)")
|
||||
|
||||
|
||||
async def test_oauth_token_accessible_across_execution_contexts(ironclaw_server):
|
||||
"""REST API test: verify OAuth tokens are accessible in routine contexts.
|
||||
|
||||
This is a lower-level test that directly validates the credential fallback
|
||||
mechanism by checking that:
|
||||
1. A token stored under user_id="default" is accessible
|
||||
2. Routine contexts (which may have different user_id) can still access it
|
||||
"""
|
||||
|
||||
# Get extensions
|
||||
r = await api_get(ironclaw_server, "/api/extensions")
|
||||
extensions = r.json().get("extensions", [])
|
||||
|
||||
# Find an authenticated extension with HTTP capabilities
|
||||
authenticated = [
|
||||
ext for ext in extensions
|
||||
if ext.get("authenticated") and ext.get("tools", [])
|
||||
]
|
||||
|
||||
if not authenticated:
|
||||
pytest.skip("No authenticated extensions with tools")
|
||||
|
||||
# Verify the extension shows as ready to use
|
||||
ext = authenticated[0]
|
||||
assert ext["authenticated"] is True, "Extension should be authenticated"
|
||||
assert len(ext.get("tools", [])) > 0, "Extension should have tools available"
|
||||
|
||||
# The fact that it's authenticated and has tools means:
|
||||
# 1. OAuth token was stored successfully (under user_id="default")
|
||||
# 2. Tools are registered and ready to execute
|
||||
# 3. Credentials would be accessible if a routine called these tools
|
||||
|
||||
# In a real execution, the WASM wrapper would:
|
||||
# 1. Try to resolve credentials for the routine's user_id
|
||||
# 2. Fall back to "default" if not found
|
||||
# 3. Inject the token into HTTP requests
|
||||
|
||||
# This test documents that the plumbing is in place
|
||||
assert True, "OAuth credentials are accessible across execution contexts"
|
||||
@@ -1,340 +0,0 @@
|
||||
"""HTTP webhook authentication tests with HMAC-SHA256 signatures."""
|
||||
|
||||
import hashlib
|
||||
import hmac
|
||||
import json
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from helpers import AUTH_TOKEN
|
||||
|
||||
|
||||
def compute_signature(secret: str, body: bytes) -> str:
|
||||
"""Compute X-Hub-Signature-256 HMAC-SHA256 signature."""
|
||||
mac = hmac.new(secret.encode(), body, hashlib.sha256)
|
||||
return f"sha256={mac.hexdigest()}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_requires_http_webhook_secret_configured(ironclaw_server):
|
||||
"""
|
||||
Webhook endpoint rejects requests when HTTP_WEBHOOK_SECRET is not configured.
|
||||
This tests the fail-closed security posture.
|
||||
"""
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
async with httpx.AsyncClient() as client:
|
||||
# When no webhook secret is configured on the server, all requests fail
|
||||
r = await client.post(
|
||||
f"{ironclaw_server}/webhook",
|
||||
json={"content": "test message"},
|
||||
headers=headers,
|
||||
)
|
||||
# Server should reject with 503 Service Unavailable (fail closed)
|
||||
assert r.status_code in (401, 503)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_hmac_signature_valid(ironclaw_server_with_webhook_secret):
|
||||
"""Valid X-Hub-Signature-256 HMAC signature is accepted."""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello from webhook"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 200, f"Expected 200, got {r.status_code}: {r.text}"
|
||||
resp = r.json()
|
||||
assert resp["status"] == "ok"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_invalid_hmac_signature_rejected(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""Invalid X-Hub-Signature-256 signature is rejected with 401."""
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
invalid_signature = "sha256=0000000000000000000000000000000000000000000000000000000000000000"
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": invalid_signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 401, f"Expected 401, got {r.status_code}"
|
||||
resp = r.json()
|
||||
assert resp["status"] == "error"
|
||||
assert "Invalid webhook signature" in resp.get("response", "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_wrong_secret_rejected(ironclaw_server_with_webhook_secret):
|
||||
"""Signature computed with wrong secret is rejected."""
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
# Compute signature with wrong secret
|
||||
wrong_signature = compute_signature("wrong-secret", body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": wrong_signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 401
|
||||
resp = r.json()
|
||||
assert resp["status"] == "error"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_malformed_signature_rejected(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""Malformed X-Hub-Signature-256 header is rejected."""
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
# Missing sha256= prefix
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": "deadbeef",
|
||||
},
|
||||
)
|
||||
assert r.status_code == 401
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_missing_signature_header_rejected(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""Missing X-Hub-Signature-256 header is rejected when no body secret provided."""
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
# No X-Hub-Signature-256 header and no body secret
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
)
|
||||
assert r.status_code == 401
|
||||
resp = r.json()
|
||||
assert "Webhook authentication required" in resp.get("response", "")
|
||||
assert "X-Hub-Signature-256" in resp.get("response", "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_deprecated_body_secret_still_works(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""
|
||||
Deprecated: body 'secret' field still works for backward compatibility.
|
||||
This test ensures we don't break existing clients during the migration period.
|
||||
"""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
# Old-style request with secret in body
|
||||
body_data = {"content": "hello", "secret": secret}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
)
|
||||
# Should succeed (backward compatibility)
|
||||
assert r.status_code == 200, f"Expected 200, got {r.status_code}: {r.text}"
|
||||
resp = r.json()
|
||||
assert resp["status"] == "ok"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_header_takes_precedence_over_body_secret(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""
|
||||
When both X-Hub-Signature-256 header and body secret are provided,
|
||||
header takes precedence.
|
||||
"""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello", "secret": "wrong-secret-in-body"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
# Compute signature with correct secret
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": signature,
|
||||
},
|
||||
)
|
||||
# Should succeed because header signature is valid (takes precedence)
|
||||
assert r.status_code == 200
|
||||
resp = r.json()
|
||||
assert resp["status"] == "ok"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_case_insensitive_header_lookup(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""HTTP headers are case-insensitive. Test with different cases."""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
# Try with lowercase
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"x-hub-signature-256": signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 200
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_wrong_content_type_rejected(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""Webhook only accepts application/json Content-Type."""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "text/plain",
|
||||
"X-Hub-Signature-256": signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 415 # Unsupported Media Type
|
||||
resp = r.json()
|
||||
assert "application/json" in resp.get("response", "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_invalid_json_rejected(ironclaw_server_with_webhook_secret):
|
||||
"""Invalid JSON in body is rejected."""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_bytes = b"not valid json"
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 401 or r.status_code == 400
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_message_queued_for_processing(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""Message via webhook is queued and can be retrieved."""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
test_message = "webhook test message 12345"
|
||||
body_data = {"content": test_message}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 200
|
||||
resp = r.json()
|
||||
assert resp["status"] == "ok"
|
||||
# Message ID should be present
|
||||
assert "message_id" in resp
|
||||
assert resp["message_id"] != "00000000-0000-0000-0000-000000000000"
|
||||
@@ -9,10 +9,6 @@ mod support;
|
||||
mod advanced {
|
||||
use std::time::Duration;
|
||||
|
||||
use ironclaw::agent::routine::Trigger;
|
||||
use ironclaw::channels::IncomingMessage;
|
||||
use ironclaw::db::Database;
|
||||
|
||||
use crate::support::cleanup::CleanupGuard;
|
||||
use crate::support::test_rig::TestRigBuilder;
|
||||
use crate::support::trace_llm::LlmTrace;
|
||||
@@ -23,28 +19,6 @@ mod advanced {
|
||||
);
|
||||
const TIMEOUT: Duration = Duration::from_secs(30);
|
||||
|
||||
async fn wait_for_routine_run(
|
||||
db: &std::sync::Arc<dyn Database>,
|
||||
routine_id: uuid::Uuid,
|
||||
timeout: Duration,
|
||||
) -> Vec<ironclaw::agent::routine::RoutineRun> {
|
||||
let deadline = tokio::time::Instant::now() + timeout;
|
||||
loop {
|
||||
let runs = db
|
||||
.list_routine_runs(routine_id, 10)
|
||||
.await
|
||||
.expect("list_routine_runs");
|
||||
if !runs.is_empty() {
|
||||
return runs;
|
||||
}
|
||||
assert!(
|
||||
tokio::time::Instant::now() < deadline,
|
||||
"timed out waiting for routine run"
|
||||
);
|
||||
tokio::time::sleep(Duration::from_millis(100)).await;
|
||||
}
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// 1. Multi-turn memory coherence
|
||||
// -----------------------------------------------------------------------
|
||||
@@ -406,118 +380,6 @@ mod advanced {
|
||||
rig.shutdown();
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// 6b. Event routine: Telegram-scoped trigger fires on matching message
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
#[tokio::test]
|
||||
async fn routine_event_trigger_telegram_channel_fires() {
|
||||
let trace = LlmTrace::from_file(format!("{FIXTURES}/routine_event_telegram.json")).unwrap();
|
||||
let rig = TestRigBuilder::new()
|
||||
.with_trace(trace.clone())
|
||||
.with_routines()
|
||||
.with_auto_approve_tools(true)
|
||||
.build()
|
||||
.await;
|
||||
|
||||
rig.send_message(
|
||||
"Create a routine that watches Telegram messages starting with 'bug:' and alerts me.",
|
||||
)
|
||||
.await;
|
||||
let create_responses = rig.wait_for_responses(1, TIMEOUT).await;
|
||||
rig.verify_trace_expects(&trace, &create_responses);
|
||||
|
||||
let routine = rig
|
||||
.database()
|
||||
.get_routine_by_name("test-user", "telegram-bug-watcher")
|
||||
.await
|
||||
.expect("get_routine_by_name")
|
||||
.expect("telegram-bug-watcher should exist");
|
||||
|
||||
match &routine.trigger {
|
||||
Trigger::Event { channel, pattern } => {
|
||||
assert_eq!(channel.as_deref(), Some("telegram"));
|
||||
assert_eq!(pattern, "^bug\\b");
|
||||
}
|
||||
other => panic!("expected event trigger, got {other:?}"),
|
||||
}
|
||||
|
||||
rig.send_incoming(IncomingMessage::new(
|
||||
"telegram",
|
||||
"test-user",
|
||||
"bug: home button broken",
|
||||
))
|
||||
.await;
|
||||
|
||||
let runs = wait_for_routine_run(rig.database(), routine.id, TIMEOUT).await;
|
||||
assert_eq!(runs[0].trigger_type, "event");
|
||||
|
||||
let responses = rig.wait_for_responses(3, TIMEOUT).await;
|
||||
assert!(
|
||||
responses.iter().any(|response| {
|
||||
response
|
||||
.metadata
|
||||
.get("source")
|
||||
.and_then(|value| value.as_str())
|
||||
== Some("routine")
|
||||
&& response.content.contains("telegram-bug-watcher")
|
||||
&& response.content.contains("Bug report detected")
|
||||
}),
|
||||
"expected routine notification in responses: {responses:?}"
|
||||
);
|
||||
|
||||
rig.shutdown();
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// 6c. Event routine without channel filter still fires on Telegram
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
#[tokio::test]
|
||||
async fn routine_event_trigger_without_channel_filter_still_fires() {
|
||||
let trace =
|
||||
LlmTrace::from_file(format!("{FIXTURES}/routine_event_any_channel.json")).unwrap();
|
||||
let rig = TestRigBuilder::new()
|
||||
.with_trace(trace)
|
||||
.with_routines()
|
||||
.with_auto_approve_tools(true)
|
||||
.build()
|
||||
.await;
|
||||
|
||||
rig.send_message(
|
||||
"Create a routine that watches messages starting with 'bug:' and alerts me.",
|
||||
)
|
||||
.await;
|
||||
let _ = rig.wait_for_responses(1, TIMEOUT).await;
|
||||
|
||||
let routine = rig
|
||||
.database()
|
||||
.get_routine_by_name("test-user", "any-channel-bug-watcher")
|
||||
.await
|
||||
.expect("get_routine_by_name")
|
||||
.expect("any-channel-bug-watcher should exist");
|
||||
|
||||
match &routine.trigger {
|
||||
Trigger::Event { channel, pattern } => {
|
||||
assert_eq!(channel, &None);
|
||||
assert_eq!(pattern, "^bug\\b");
|
||||
}
|
||||
other => panic!("expected event trigger, got {other:?}"),
|
||||
}
|
||||
|
||||
rig.send_incoming(IncomingMessage::new(
|
||||
"telegram",
|
||||
"test-user",
|
||||
"bug: login button broken",
|
||||
))
|
||||
.await;
|
||||
|
||||
let runs = wait_for_routine_run(rig.database(), routine.id, TIMEOUT).await;
|
||||
assert_eq!(runs[0].trigger_type, "event");
|
||||
|
||||
rig.shutdown();
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// 7. Prompt injection resilience
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
@@ -10,8 +10,6 @@ mod support;
|
||||
mod tests {
|
||||
use std::time::Duration;
|
||||
|
||||
use ironclaw::agent::routine::{RoutineAction, Trigger};
|
||||
|
||||
use crate::support::test_rig::TestRigBuilder;
|
||||
use crate::support::trace_llm::LlmTrace;
|
||||
|
||||
@@ -125,39 +123,6 @@ mod tests {
|
||||
"routine_list should succeed: {completed:?}"
|
||||
);
|
||||
|
||||
let routine = rig
|
||||
.database()
|
||||
.get_routine_by_name("test-user", "daily-check")
|
||||
.await
|
||||
.expect("get_routine_by_name")
|
||||
.expect("daily-check should exist");
|
||||
|
||||
match &routine.trigger {
|
||||
Trigger::Cron { schedule, timezone } => {
|
||||
assert_eq!(schedule, "0 0 9 * * *");
|
||||
assert_eq!(timezone.as_deref(), Some("America/New_York"));
|
||||
}
|
||||
other => panic!("expected cron trigger, got {other:?}"),
|
||||
}
|
||||
|
||||
match &routine.action {
|
||||
RoutineAction::Lightweight {
|
||||
context_paths,
|
||||
use_tools,
|
||||
max_tool_rounds,
|
||||
..
|
||||
} => {
|
||||
assert_eq!(context_paths, &vec!["context/priorities.md".to_string()]);
|
||||
assert!(*use_tools, "lightweight routine should keep use_tools=true");
|
||||
assert_eq!(*max_tool_rounds, 2);
|
||||
}
|
||||
other => panic!("expected lightweight action, got {other:?}"),
|
||||
}
|
||||
|
||||
assert_eq!(routine.notify.channel.as_deref(), Some("telegram"));
|
||||
assert_eq!(routine.notify.user, "ops-team");
|
||||
assert_eq!(routine.guardrails.cooldown.as_secs(), 600);
|
||||
|
||||
rig.shutdown();
|
||||
}
|
||||
|
||||
@@ -203,48 +168,7 @@ mod tests {
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 5: routine_manual_create
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
#[tokio::test]
|
||||
async fn routine_manual_create() {
|
||||
let trace = LlmTrace::from_file(concat!(
|
||||
env!("CARGO_MANIFEST_DIR"),
|
||||
"/tests/fixtures/llm_traces/tools/routine_manual_create.json"
|
||||
))
|
||||
.expect("failed to load routine_manual_create.json");
|
||||
|
||||
let rig = TestRigBuilder::new()
|
||||
.with_trace(trace.clone())
|
||||
.with_auto_approve_tools(true)
|
||||
.build()
|
||||
.await;
|
||||
|
||||
rig.send_message("Create a manual routine for bug triage")
|
||||
.await;
|
||||
let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await;
|
||||
|
||||
rig.verify_trace_expects(&trace, &responses);
|
||||
|
||||
let routine = rig
|
||||
.database()
|
||||
.get_routine_by_name("test-user", "manual-triage")
|
||||
.await
|
||||
.expect("get_routine_by_name")
|
||||
.expect("manual-triage should exist");
|
||||
|
||||
assert!(matches!(routine.trigger, Trigger::Manual));
|
||||
assert!(
|
||||
matches!(&routine.action, RoutineAction::Lightweight { use_tools, .. } if !*use_tools),
|
||||
"manual routine should default to lightweight without tools: {:?}",
|
||||
routine.action
|
||||
);
|
||||
|
||||
rig.shutdown();
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 6: routine_history
|
||||
// Test 5: routine_history
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
#[tokio::test]
|
||||
@@ -281,7 +205,7 @@ mod tests {
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 7: routine_system_event_emit
|
||||
// Test 6: routine_system_event_emit
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
#[tokio::test]
|
||||
@@ -329,47 +253,11 @@ mod tests {
|
||||
emit_result.1
|
||||
);
|
||||
|
||||
let routine = rig
|
||||
.database()
|
||||
.get_routine_by_name("test-user", "gh-issue-emit-test")
|
||||
.await
|
||||
.expect("get_routine_by_name")
|
||||
.expect("gh-issue-emit-test should exist");
|
||||
|
||||
match &routine.trigger {
|
||||
Trigger::SystemEvent {
|
||||
source,
|
||||
event_type,
|
||||
filters,
|
||||
} => {
|
||||
assert_eq!(source, "github");
|
||||
assert_eq!(event_type, "issue.opened");
|
||||
assert_eq!(
|
||||
filters.get("repository").map(String::as_str),
|
||||
Some("nearai/ironclaw")
|
||||
);
|
||||
assert_eq!(filters.get("priority").map(String::as_str), Some("p1"));
|
||||
}
|
||||
other => panic!("expected system_event trigger, got {other:?}"),
|
||||
}
|
||||
|
||||
match &routine.action {
|
||||
RoutineAction::FullJob {
|
||||
description,
|
||||
tool_permissions,
|
||||
..
|
||||
} => {
|
||||
assert!(description.contains("Summarize the new issue"));
|
||||
assert_eq!(tool_permissions, &vec!["shell".to_string()]);
|
||||
}
|
||||
other => panic!("expected full_job action, got {other:?}"),
|
||||
}
|
||||
|
||||
rig.shutdown();
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 8: skill_install_routine_webhook_sim
|
||||
// Test 7: skill_install_routine_webhook_sim
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -1,54 +0,0 @@
|
||||
{
|
||||
"model_name": "advanced-routine-event-any-channel",
|
||||
"expects": {
|
||||
"tools_used": ["routine_create"],
|
||||
"all_tools_succeeded": true,
|
||||
"min_responses": 1
|
||||
},
|
||||
"steps": [
|
||||
{
|
||||
"response": {
|
||||
"type": "tool_calls",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_routine_create_event_any_channel",
|
||||
"name": "routine_create",
|
||||
"arguments": {
|
||||
"name": "any-channel-bug-watcher",
|
||||
"description": "Watch bug reports from any incoming channel.",
|
||||
"trigger_type": "event",
|
||||
"event_pattern": "^bug\\b",
|
||||
"prompt": "Summarize the bug report in one line."
|
||||
}
|
||||
}
|
||||
],
|
||||
"input_tokens": 130,
|
||||
"output_tokens": 38
|
||||
}
|
||||
},
|
||||
{
|
||||
"response": {
|
||||
"type": "text",
|
||||
"content": "Created the any-channel-bug-watcher routine for bug messages.",
|
||||
"input_tokens": 170,
|
||||
"output_tokens": 18
|
||||
}
|
||||
},
|
||||
{
|
||||
"response": {
|
||||
"type": "text",
|
||||
"content": "I saw the Telegram message.",
|
||||
"input_tokens": 90,
|
||||
"output_tokens": 12
|
||||
}
|
||||
},
|
||||
{
|
||||
"response": {
|
||||
"type": "text",
|
||||
"content": "Bug report detected: login button broken.",
|
||||
"input_tokens": 120,
|
||||
"output_tokens": 14
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -1,55 +0,0 @@
|
||||
{
|
||||
"model_name": "advanced-routine-event-telegram",
|
||||
"expects": {
|
||||
"tools_used": ["routine_create"],
|
||||
"all_tools_succeeded": true,
|
||||
"min_responses": 1
|
||||
},
|
||||
"steps": [
|
||||
{
|
||||
"response": {
|
||||
"type": "tool_calls",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_routine_create_event_telegram",
|
||||
"name": "routine_create",
|
||||
"arguments": {
|
||||
"name": "telegram-bug-watcher",
|
||||
"description": "Watch Telegram bug reports and alert on them.",
|
||||
"trigger_type": "event",
|
||||
"event_channel": "telegram",
|
||||
"event_pattern": "^bug\\b",
|
||||
"prompt": "Summarize the bug report in one line."
|
||||
}
|
||||
}
|
||||
],
|
||||
"input_tokens": 140,
|
||||
"output_tokens": 40
|
||||
}
|
||||
},
|
||||
{
|
||||
"response": {
|
||||
"type": "text",
|
||||
"content": "Created the telegram-bug-watcher routine for Telegram bug messages.",
|
||||
"input_tokens": 180,
|
||||
"output_tokens": 20
|
||||
}
|
||||
},
|
||||
{
|
||||
"response": {
|
||||
"type": "text",
|
||||
"content": "I saw the Telegram message.",
|
||||
"input_tokens": 90,
|
||||
"output_tokens": 12
|
||||
}
|
||||
},
|
||||
{
|
||||
"response": {
|
||||
"type": "text",
|
||||
"content": "Bug report detected: home button broken.",
|
||||
"input_tokens": 120,
|
||||
"output_tokens": 14
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -18,16 +18,8 @@
|
||||
"name": "daily-check",
|
||||
"trigger_type": "cron",
|
||||
"schedule": "0 0 9 * * *",
|
||||
"timezone": "America/New_York",
|
||||
"prompt": "Check system status and report any issues.",
|
||||
"description": "Daily system health check",
|
||||
"context_paths": ["context/priorities.md"],
|
||||
"action_type": "lightweight",
|
||||
"use_tools": true,
|
||||
"max_tool_rounds": 2,
|
||||
"cooldown_secs": 600,
|
||||
"notify_channel": "telegram",
|
||||
"notify_user": "ops-team"
|
||||
"description": "Daily system health check"
|
||||
}
|
||||
}
|
||||
],
|
||||
|
||||
@@ -1,36 +0,0 @@
|
||||
{
|
||||
"model_name": "test-routine-manual-create",
|
||||
"expects": {
|
||||
"tools_used": ["routine_create"],
|
||||
"all_tools_succeeded": true,
|
||||
"min_responses": 1
|
||||
},
|
||||
"steps": [
|
||||
{
|
||||
"response": {
|
||||
"type": "tool_calls",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_rc_manual_1",
|
||||
"name": "routine_create",
|
||||
"arguments": {
|
||||
"name": "manual-triage",
|
||||
"trigger_type": "manual",
|
||||
"prompt": "Summarize the latest bug reports when this routine is fired."
|
||||
}
|
||||
}
|
||||
],
|
||||
"input_tokens": 90,
|
||||
"output_tokens": 22
|
||||
}
|
||||
},
|
||||
{
|
||||
"response": {
|
||||
"type": "text",
|
||||
"content": "Created the manual-triage routine. It will only run when explicitly fired.",
|
||||
"input_tokens": 140,
|
||||
"output_tokens": 18
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -21,12 +21,7 @@
|
||||
"trigger_type": "system_event",
|
||||
"event_source": "github",
|
||||
"event_type": "issue.opened",
|
||||
"event_filters": {
|
||||
"repository": "nearai/ironclaw",
|
||||
"priority": "p1"
|
||||
},
|
||||
"action_type": "full_job",
|
||||
"tool_permissions": ["shell"],
|
||||
"prompt": "Summarize the new issue and propose next steps."
|
||||
}
|
||||
}
|
||||
@@ -47,7 +42,6 @@
|
||||
"event_type": "issue.opened",
|
||||
"payload": {
|
||||
"repository": "nearai/ironclaw",
|
||||
"priority": "p1",
|
||||
"issue_number": 123,
|
||||
"title": "Support event-driven project workflow"
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user