Compare commits

..
21 changed files with 333 additions and 1520 deletions
-18
View File
@@ -7,24 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased] ## [Unreleased]
## [0.23.0](https://github.com/nearai/ironclaw/compare/ironclaw-v0.22.0...ironclaw-v0.23.0) - 2026-03-27
### Added
- complete multi-tenant isolation — phases 24 ([#1614](https://github.com/nearai/ironclaw/pull/1614))
### Fixed
- *(routines)* recover delete name after failed update fallback ([#1108](https://github.com/nearai/ironclaw/pull/1108))
- *(mcp)* handle 202 Accepted and wire session manager for Streamable HTTP ([#1437](https://github.com/nearai/ironclaw/pull/1437))
- *(extensions)* channel-relay auth dead-end, observability, and URL override ([#1681](https://github.com/nearai/ironclaw/pull/1681))
- *(agent)* discard truncated tool calls when finish_reason == Length ([#1631](https://github.com/nearai/ironclaw/pull/1631)) ([#1632](https://github.com/nearai/ironclaw/pull/1632))
- *(llm)* filter XML tool-call recovery by context ([#1641](https://github.com/nearai/ironclaw/pull/1641))
### Other
- Support direct hosted OAuth callbacks with proxy auth token ([#1684](https://github.com/nearai/ironclaw/pull/1684))
## [0.22.0](https://github.com/nearai/ironclaw/compare/ironclaw-v0.21.0...ironclaw-v0.22.0) - 2026-03-25 ## [0.22.0](https://github.com/nearai/ironclaw/compare/ironclaw-v0.21.0...ironclaw-v0.22.0) - 2026-03-25
### Added ### Added
Generated
+1 -1
View File
@@ -3390,7 +3390,7 @@ dependencies = [
[[package]] [[package]]
name = "ironclaw" name = "ironclaw"
version = "0.23.0" version = "0.22.0"
dependencies = [ dependencies = [
"aes-gcm", "aes-gcm",
"aho-corasick", "aho-corasick",
+1 -1
View File
@@ -20,7 +20,7 @@ exclude = [
[package] [package]
name = "ironclaw" name = "ironclaw"
version = "0.23.0" version = "0.22.0"
edition = "2024" edition = "2024"
rust-version = "1.92" rust-version = "1.92"
description = "Secure personal AI assistant that protects your data and expands its capabilities on the fly" description = "Secure personal AI assistant that protects your data and expands its capabilities on the fly"
+6 -61
View File
@@ -122,32 +122,18 @@ impl RelayClient {
/// instance_url in chat-api. IronClaw only passes an optional CSRF nonce /// instance_url in chat-api. IronClaw only passes an optional CSRF nonce
/// for validating the callback — no URLs. /// for validating the callback — no URLs.
pub async fn initiate_oauth(&self, state_nonce: Option<&str>) -> Result<String, RelayError> { pub async fn initiate_oauth(&self, state_nonce: Option<&str>) -> Result<String, RelayError> {
let url = format!("{}/oauth/slack/auth", self.base_url);
tracing::debug!(relay_url = %url, "RelayClient::initiate_oauth: sending request");
let mut query: Vec<(&str, &str)> = vec![]; let mut query: Vec<(&str, &str)> = vec![];
if let Some(nonce) = state_nonce { if let Some(nonce) = state_nonce {
query.push(("state_nonce", nonce)); query.push(("state_nonce", nonce));
} }
let resp = self let resp = self
.http .http
.get(&url) .get(format!("{}/oauth/slack/auth", self.base_url))
.bearer_auth(self.api_key.expose_secret()) .bearer_auth(self.api_key.expose_secret())
.query(&query) .query(&query)
.send() .send()
.await .await
.map_err(|e| { .map_err(|e| RelayError::Network(e.to_string()))?;
tracing::warn!(
relay_url = %url,
error = %e,
"RelayClient::initiate_oauth: network request failed"
);
RelayError::Network(e.to_string())
})?;
tracing::debug!(
relay_url = %url,
status = %resp.status(),
"RelayClient::initiate_oauth: received response"
);
let status = resp.status(); let status = resp.status();
if status.is_redirection() { if status.is_redirection() {
@@ -238,39 +224,20 @@ impl RelayClient {
method: &str, method: &str,
body: serde_json::Value, body: serde_json::Value,
) -> Result<serde_json::Value, RelayError> { ) -> Result<serde_json::Value, RelayError> {
let url = format!("{}/proxy/{}/{}", self.base_url, provider, method);
tracing::debug!(
relay_url = %url,
provider = %provider,
method = %method,
"RelayClient::proxy_provider: sending request"
);
let query: Vec<(&str, &str)> = vec![("team_id", team_id)]; let query: Vec<(&str, &str)> = vec![("team_id", team_id)];
let resp = self let resp = self
.http .http
.post(&url) .post(format!("{}/proxy/{}/{}", self.base_url, provider, method))
.bearer_auth(self.api_key.expose_secret()) .bearer_auth(self.api_key.expose_secret())
.query(&query) .query(&query)
.json(&body) .json(&body)
.send() .send()
.await .await
.map_err(|e| { .map_err(|e| RelayError::Network(e.to_string()))?;
tracing::warn!(
relay_url = %url,
error = %e,
"RelayClient::proxy_provider: network request failed"
);
RelayError::Network(e.to_string())
})?;
if !resp.status().is_success() { if !resp.status().is_success() {
let status = resp.status().as_u16(); let status = resp.status().as_u16();
let body = resp.text().await.unwrap_or_default(); let body = resp.text().await.unwrap_or_default();
tracing::warn!(
relay_url = %url,
status = status,
"RelayClient::proxy_provider: channel-relay returned error"
);
return Err(RelayError::Api { return Err(RelayError::Api {
status, status,
message: body, message: body,
@@ -288,45 +255,23 @@ impl RelayClient {
/// 32-byte secret. Called once at activation time; the result is cached in the /// 32-byte secret. Called once at activation time; the result is cached in the
/// extension manager so subsequent calls to `relay_signing_secret()` use it. /// extension manager so subsequent calls to `relay_signing_secret()` use it.
pub async fn get_signing_secret(&self, team_id: &str) -> Result<Vec<u8>, RelayError> { pub async fn get_signing_secret(&self, team_id: &str) -> Result<Vec<u8>, RelayError> {
let url = format!("{}/relay/signing-secret", self.base_url);
tracing::debug!(
relay_url = %url,
"RelayClient::get_signing_secret: fetching signing secret"
);
let resp = self let resp = self
.http .http
.get(&url) .get(format!("{}/relay/signing-secret", self.base_url))
.bearer_auth(self.api_key.expose_secret()) .bearer_auth(self.api_key.expose_secret())
.query(&[("team_id", team_id)]) .query(&[("team_id", team_id)])
.send() .send()
.await .await
.map_err(|e| { .map_err(|e| RelayError::Network(e.to_string()))?;
tracing::warn!(
relay_url = %url,
error = %e,
"RelayClient::get_signing_secret: network request failed"
);
RelayError::Network(e.to_string())
})?;
if !resp.status().is_success() { if !resp.status().is_success() {
let status = resp.status().as_u16(); let status = resp.status().as_u16();
let body = resp.text().await.unwrap_or_default(); let body = resp.text().await.unwrap_or_default();
tracing::warn!(
relay_url = %url,
status = status,
body = %body,
"RelayClient::get_signing_secret: channel-relay returned error"
);
return Err(RelayError::Api { return Err(RelayError::Api {
status, status,
message: body, message: body,
}); });
} }
tracing::debug!(
relay_url = %url,
"RelayClient::get_signing_secret: received successful response"
);
let body: serde_json::Value = resp let body: serde_json::Value = resp
.json() .json()
+5 -484
View File
@@ -836,10 +836,10 @@ async fn oauth_callback_handler(
let result: Result<(), String> = async { let result: Result<(), String> = async {
let token_response = if let Some(proxy_url) = &exchange_proxy_url { let token_response = if let Some(proxy_url) = &exchange_proxy_url {
let oauth_proxy_auth_token = flow.oauth_proxy_auth_token().unwrap_or_default(); let gateway_token = flow.gateway_token.as_deref().unwrap_or_default();
oauth_defaults::exchange_via_proxy(oauth_defaults::ProxyTokenExchangeRequest { oauth_defaults::exchange_via_proxy(oauth_defaults::ProxyTokenExchangeRequest {
proxy_url, proxy_url,
gateway_token: oauth_proxy_auth_token, gateway_token,
token_url: &flow.token_url, token_url: &flow.token_url,
client_id: &flow.client_id, client_id: &flow.client_id,
client_secret: flow.client_secret.as_deref(), client_secret: flow.client_secret.as_deref(),
@@ -1177,31 +1177,11 @@ async fn slack_relay_oauth_callback_handler(
// Store team_id in settings // Store team_id in settings
let team_id_key = format!("relay:{}:team_id", DEFAULT_RELAY_NAME); let team_id_key = format!("relay:{}:team_id", DEFAULT_RELAY_NAME);
tracing::info!( let _ = store
relay = DEFAULT_RELAY_NAME,
owner_id = %state.owner_id,
team_id_key = %team_id_key,
"relay OAuth callback: storing team_id in settings"
);
store
.set_setting(&state.owner_id, &team_id_key, &serde_json::json!(team_id)) .set_setting(&state.owner_id, &team_id_key, &serde_json::json!(team_id))
.await .await;
.map_err(|e| {
tracing::error!(
relay = DEFAULT_RELAY_NAME,
owner_id = %state.owner_id,
error = %e,
"relay OAuth callback: failed to persist team_id to settings store"
);
format!("Failed to persist relay team_id: {e}")
})?;
// Activate the relay channel // Activate the relay channel
tracing::info!(
relay = DEFAULT_RELAY_NAME,
owner_id = %state.owner_id,
"relay OAuth callback: activating relay channel"
);
ext_mgr ext_mgr
.activate_stored_relay(DEFAULT_RELAY_NAME, &state.owner_id) .activate_stored_relay(DEFAULT_RELAY_NAME, &state.owner_id)
.await .await
@@ -2201,11 +2181,6 @@ async fn extensions_activate_handler(
AuthenticatedUser(user): AuthenticatedUser, AuthenticatedUser(user): AuthenticatedUser,
Path(name): Path<String>, Path(name): Path<String>,
) -> Result<Json<ActionResponse>, (StatusCode, String)> { ) -> Result<Json<ActionResponse>, (StatusCode, String)> {
tracing::debug!(
extension = %name,
user_id = %user.user_id,
"extensions_activate_handler: received activate request"
);
let ext_mgr = state.extension_manager.as_ref().ok_or(( let ext_mgr = state.extension_manager.as_ref().ok_or((
StatusCode::NOT_IMPLEMENTED, StatusCode::NOT_IMPLEMENTED,
"Extension manager not available (secrets store required)".to_string(), "Extension manager not available (secrets store required)".to_string(),
@@ -2213,10 +2188,6 @@ async fn extensions_activate_handler(
match ext_mgr.activate(&name, &user.user_id).await { match ext_mgr.activate(&name, &user.user_id).await {
Ok(result) => { Ok(result) => {
tracing::info!(
extension = %name,
"extensions_activate_handler: activation succeeded"
);
// Activation loaded the WASM module. Check if the tool needs // Activation loaded the WASM module. Check if the tool needs
// OAuth scope expansion (e.g., adding google-docs when gmail // OAuth scope expansion (e.g., adding google-docs when gmail
// already has a token but missing the documents scope). // already has a token but missing the documents scope).
@@ -2235,13 +2206,6 @@ async fn extensions_activate_handler(
crate::extensions::ExtensionError::AuthRequired crate::extensions::ExtensionError::AuthRequired
); );
tracing::debug!(
extension = %name,
error = %activate_err,
needs_auth = needs_auth,
"extensions_activate_handler: activation failed, attempting auth fallback"
);
if !needs_auth { if !needs_auth {
return Ok(Json(ActionResponse::fail(activate_err.to_string()))); return Ok(Json(ActionResponse::fail(activate_err.to_string())));
} }
@@ -2249,21 +2213,10 @@ async fn extensions_activate_handler(
// Activation failed due to auth; try authenticating first. // Activation failed due to auth; try authenticating first.
match ext_mgr.auth(&name, &user.user_id).await { match ext_mgr.auth(&name, &user.user_id).await {
Ok(auth_result) if auth_result.is_authenticated() => { Ok(auth_result) if auth_result.is_authenticated() => {
tracing::debug!(
extension = %name,
"extensions_activate_handler: auth reports authenticated, retrying activate"
);
// Auth succeeded, retry activation. // Auth succeeded, retry activation.
match ext_mgr.activate(&name, &user.user_id).await { match ext_mgr.activate(&name, &user.user_id).await {
Ok(result) => Ok(Json(ActionResponse::ok(result.message))), Ok(result) => Ok(Json(ActionResponse::ok(result.message))),
Err(e) => { Err(e) => Ok(Json(ActionResponse::fail(e.to_string()))),
tracing::warn!(
extension = %name,
error = %e,
"extensions_activate_handler: retry after auth still failed"
);
Ok(Json(ActionResponse::fail(e.to_string())))
}
} }
} }
Ok(auth_result) => { Ok(auth_result) => {
@@ -3057,160 +3010,6 @@ mod tests {
.with_state(state) .with_state(state)
} }
#[derive(Clone, Debug)]
struct RecordedOauthProxyRequest {
authorization: Option<String>,
form: std::collections::HashMap<String, String>,
}
#[derive(Clone)]
struct MockOauthProxyState {
requests: Arc<tokio::sync::Mutex<Vec<RecordedOauthProxyRequest>>>,
}
struct MockOauthProxyServer {
addr: std::net::SocketAddr,
requests: Arc<tokio::sync::Mutex<Vec<RecordedOauthProxyRequest>>>,
shutdown_tx: Option<tokio::sync::oneshot::Sender<()>>,
server_task: Option<tokio::task::JoinHandle<()>>,
}
impl MockOauthProxyServer {
async fn start() -> Self {
async fn exchange_handler(
State(state): State<MockOauthProxyState>,
headers: axum::http::HeaderMap,
axum::Form(form): axum::Form<std::collections::HashMap<String, String>>,
) -> Json<serde_json::Value> {
state.requests.lock().await.push(RecordedOauthProxyRequest {
authorization: headers
.get(axum::http::header::AUTHORIZATION)
.and_then(|value| value.to_str().ok())
.map(str::to_string),
form,
});
Json(serde_json::json!({
"access_token": "proxy-access-token",
"refresh_token": "proxy-refresh-token",
"expires_in": 7200
}))
}
let requests = Arc::new(tokio::sync::Mutex::new(Vec::new()));
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
.await
.expect("bind mock oauth proxy");
let addr = listener.local_addr().expect("mock oauth proxy addr");
let app = Router::new()
.route("/oauth/exchange", post(exchange_handler))
.with_state(MockOauthProxyState {
requests: Arc::clone(&requests),
});
let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel::<()>();
let server_task = tokio::spawn(async move {
let _ = axum::serve(listener, app)
.with_graceful_shutdown(async {
let _ = shutdown_rx.await;
})
.await;
});
Self {
addr,
requests,
shutdown_tx: Some(shutdown_tx),
server_task: Some(server_task),
}
}
fn base_url(&self) -> String {
format!("http://{}", self.addr)
}
async fn requests(&self) -> Vec<RecordedOauthProxyRequest> {
self.requests.lock().await.clone()
}
async fn shutdown(mut self) {
if let Some(tx) = self.shutdown_tx.take() {
let _ = tx.send(());
}
if let Some(task) = self.server_task.take() {
let _ = task.await;
}
}
}
impl Drop for MockOauthProxyServer {
fn drop(&mut self) {
if let Some(tx) = self.shutdown_tx.take() {
let _ = tx.send(());
}
if let Some(task) = self.server_task.take() {
task.abort();
}
}
}
struct EnvVarGuard {
key: &'static str,
original: Option<String>,
}
impl Drop for EnvVarGuard {
fn drop(&mut self) {
// SAFETY: Tests use lock_env() to serialize environment access.
unsafe {
if let Some(ref value) = self.original {
std::env::set_var(self.key, value);
} else {
std::env::remove_var(self.key);
}
}
}
}
fn set_env_var(key: &'static str, value: Option<&str>) -> EnvVarGuard {
let original = std::env::var(key).ok();
// SAFETY: Tests use lock_env() to serialize environment access.
unsafe {
if let Some(value) = value {
std::env::set_var(key, value);
} else {
std::env::remove_var(key);
}
}
EnvVarGuard { key, original }
}
fn fresh_pending_oauth_flow(
secrets: Arc<dyn crate::secrets::SecretsStore + Send + Sync>,
sse_manager: Option<Arc<SseManager>>,
oauth_proxy_auth_token: Option<String>,
) -> crate::cli::oauth_defaults::PendingOAuthFlow {
crate::cli::oauth_defaults::PendingOAuthFlow {
extension_name: "test_tool".to_string(),
display_name: "Test Tool".to_string(),
token_url: "https://example.com/token".to_string(),
client_id: "client123".to_string(),
client_secret: None,
redirect_uri: "https://example.com/oauth/callback".to_string(),
code_verifier: Some("test-code-verifier".to_string()),
access_token_field: "access_token".to_string(),
secret_name: "test_token".to_string(),
provider: Some("google".to_string()),
validation_endpoint: None,
scopes: vec!["email".to_string()],
user_id: "test".to_string(),
secrets,
sse_manager,
gateway_token: oauth_proxy_auth_token,
token_exchange_extra_params: std::collections::HashMap::new(),
client_id_secret_name: None,
created_at: std::time::Instant::now(),
}
}
#[tokio::test] #[tokio::test]
async fn test_extensions_setup_submit_returns_failure_when_not_activated() { async fn test_extensions_setup_submit_returns_failure_when_not_activated() {
use axum::body::Body; use axum::body::Body;
@@ -3868,284 +3667,6 @@ mod tests {
); );
} }
#[tokio::test]
async fn test_oauth_callback_accepts_versioned_hosted_state_without_instance_name() {
use axum::body::Body;
use tower::ServiceExt;
let secrets: Arc<dyn crate::secrets::SecretsStore + Send + Sync> =
Arc::new(crate::secrets::InMemorySecretsStore::new(Arc::new(
crate::secrets::SecretsCrypto::new(secrecy::SecretString::from(
TEST_GATEWAY_CRYPTO_KEY.to_string(),
))
.expect("crypto"),
)));
let (ext_mgr, _wasm_tools_dir, _wasm_channels_dir) = test_ext_mgr(secrets.clone());
let Some(created_at) = expired_flow_created_at() else {
eprintln!(
"Skipping versioned OAuth state without instance test: monotonic uptime below expiry window"
);
return;
};
let flow = crate::cli::oauth_defaults::PendingOAuthFlow {
extension_name: "test_tool".to_string(),
display_name: "Test Tool".to_string(),
token_url: "https://example.com/token".to_string(),
client_id: "client123".to_string(),
client_secret: None,
redirect_uri: "https://example.com/oauth/callback".to_string(),
code_verifier: None,
access_token_field: "access_token".to_string(),
secret_name: "test_token".to_string(),
provider: None,
validation_endpoint: None,
scopes: vec![],
user_id: "test".to_string(),
secrets,
sse_manager: None,
gateway_token: None,
token_exchange_extra_params: std::collections::HashMap::new(),
client_id_secret_name: None,
created_at,
};
ext_mgr
.pending_oauth_flows()
.write()
.await
.insert("test_nonce".to_string(), flow);
let state = test_gateway_state(Some(ext_mgr.clone()));
let app = test_oauth_router(state);
let versioned_state =
crate::cli::oauth_defaults::encode_hosted_oauth_state("test_nonce", None);
let req = axum::http::Request::builder()
.uri(format!(
"/oauth/callback?code=fake_code&state={}",
urlencoding::encode(&versioned_state)
))
.body(Body::empty())
.expect("request");
let resp = ServiceExt::<axum::http::Request<Body>>::oneshot(app, req)
.await
.expect("response");
assert_eq!(resp.status(), StatusCode::OK);
let body = axum::body::to_bytes(resp.into_body(), 1024 * 64)
.await
.expect("body");
let html = String::from_utf8_lossy(&body);
assert!(html.contains("Authorization Failed"));
assert!(
ext_mgr
.pending_oauth_flows()
.read()
.await
.get("test_nonce")
.is_none()
);
}
#[allow(clippy::await_holding_lock)]
#[tokio::test]
async fn test_oauth_callback_happy_path_with_gateway_token_fallback() {
use axum::body::Body;
use tower::ServiceExt;
let proxy = MockOauthProxyServer::start().await;
// Keep the process-wide env locked for the full callback so the handler
// sees a stable proxy URL/token configuration throughout the test.
let _env_guard = crate::config::helpers::lock_env();
let _exchange_url_guard =
set_env_var("IRONCLAW_OAUTH_EXCHANGE_URL", Some(&proxy.base_url()));
let _proxy_auth_guard = set_env_var("IRONCLAW_OAUTH_PROXY_AUTH_TOKEN", None);
let _gateway_token_guard = set_env_var("GATEWAY_AUTH_TOKEN", Some("gateway-test-token"));
let secrets = test_secrets_store();
let (ext_mgr, _wasm_tools_dir, _wasm_channels_dir) = test_ext_mgr(Arc::clone(&secrets));
let sse_mgr = Arc::new(SseManager::new());
let mut receiver = sse_mgr.sender().subscribe();
let flow = fresh_pending_oauth_flow(
Arc::clone(&secrets),
Some(Arc::clone(&sse_mgr)),
crate::cli::oauth_defaults::oauth_proxy_auth_token(),
);
ext_mgr
.pending_oauth_flows()
.write()
.await
.insert("test_nonce".to_string(), flow);
let state = test_gateway_state(Some(ext_mgr.clone()));
let app = test_oauth_router(state);
let versioned_state =
crate::cli::oauth_defaults::encode_hosted_oauth_state("test_nonce", Some("myinstance"));
let req = axum::http::Request::builder()
.uri(format!(
"/oauth/callback?code=fake_code&state={}",
urlencoding::encode(&versioned_state)
))
.body(Body::empty())
.expect("request");
let resp = ServiceExt::<axum::http::Request<Body>>::oneshot(app, req)
.await
.expect("response");
assert_eq!(resp.status(), StatusCode::OK);
let body = axum::body::to_bytes(resp.into_body(), 1024 * 64)
.await
.expect("body");
let html = String::from_utf8_lossy(&body);
assert!(html.contains("Test Tool Connected"));
let requests = proxy.requests().await;
assert_eq!(requests.len(), 1);
assert_eq!(
requests[0].authorization.as_deref(),
Some("Bearer gateway-test-token")
);
assert_eq!(
requests[0].form.get("code").map(String::as_str),
Some("fake_code")
);
assert_eq!(
requests[0].form.get("code_verifier").map(String::as_str),
Some("test-code-verifier")
);
let access_token = secrets
.get_decrypted("test", "test_token")
.await
.expect("access token stored");
assert_eq!(access_token.expose(), "proxy-access-token");
let refresh_token = secrets
.get_decrypted("test", "test_token_refresh_token")
.await
.expect("refresh token stored");
assert_eq!(refresh_token.expose(), "proxy-refresh-token");
match receiver.recv().await.expect("auth_completed event").event {
crate::channels::web::types::AppEvent::AuthCompleted {
extension_name,
success,
..
} => {
assert_eq!(extension_name, "test_tool");
assert!(success, "OAuth callback should broadcast success");
}
event => panic!("expected AuthCompleted event, got {event:?}"),
}
proxy.shutdown().await;
}
#[allow(clippy::await_holding_lock)]
#[tokio::test]
async fn test_oauth_callback_happy_path_with_dedicated_proxy_auth_token() {
use axum::body::Body;
use tower::ServiceExt;
let proxy = MockOauthProxyServer::start().await;
// Keep the process-wide env locked for the full callback so the handler
// sees a stable proxy URL/token configuration throughout the test.
let _env_guard = crate::config::helpers::lock_env();
let _exchange_url_guard =
set_env_var("IRONCLAW_OAUTH_EXCHANGE_URL", Some(&proxy.base_url()));
let _proxy_auth_guard = set_env_var(
"IRONCLAW_OAUTH_PROXY_AUTH_TOKEN",
Some("shared-oauth-proxy-secret"),
);
let _gateway_token_guard = set_env_var("GATEWAY_AUTH_TOKEN", None);
let secrets = test_secrets_store();
let (ext_mgr, _wasm_tools_dir, _wasm_channels_dir) = test_ext_mgr(Arc::clone(&secrets));
let sse_mgr = Arc::new(SseManager::new());
let mut receiver = sse_mgr.sender().subscribe();
let flow = fresh_pending_oauth_flow(
Arc::clone(&secrets),
Some(Arc::clone(&sse_mgr)),
crate::cli::oauth_defaults::oauth_proxy_auth_token(),
);
ext_mgr
.pending_oauth_flows()
.write()
.await
.insert("test_nonce".to_string(), flow);
let state = test_gateway_state(Some(ext_mgr.clone()));
let app = test_oauth_router(state);
let versioned_state =
crate::cli::oauth_defaults::encode_hosted_oauth_state("test_nonce", None);
let req = axum::http::Request::builder()
.uri(format!(
"/oauth/callback?code=fake_code&state={}",
urlencoding::encode(&versioned_state)
))
.body(Body::empty())
.expect("request");
let resp = ServiceExt::<axum::http::Request<Body>>::oneshot(app, req)
.await
.expect("response");
assert_eq!(resp.status(), StatusCode::OK);
let body = axum::body::to_bytes(resp.into_body(), 1024 * 64)
.await
.expect("body");
let html = String::from_utf8_lossy(&body);
assert!(html.contains("Test Tool Connected"));
let requests = proxy.requests().await;
assert_eq!(requests.len(), 1);
assert_eq!(
requests[0].authorization.as_deref(),
Some("Bearer shared-oauth-proxy-secret")
);
assert_eq!(
requests[0].form.get("code").map(String::as_str),
Some("fake_code")
);
assert_eq!(
requests[0].form.get("code_verifier").map(String::as_str),
Some("test-code-verifier")
);
let access_token = secrets
.get_decrypted("test", "test_token")
.await
.expect("access token stored");
assert_eq!(access_token.expose(), "proxy-access-token");
let refresh_token = secrets
.get_decrypted("test", "test_token_refresh_token")
.await
.expect("refresh token stored");
assert_eq!(refresh_token.expose(), "proxy-refresh-token");
match receiver.recv().await.expect("auth_completed event").event {
crate::channels::web::types::AppEvent::AuthCompleted {
extension_name,
success,
..
} => {
assert_eq!(extension_name, "test_tool");
assert!(success, "OAuth callback should broadcast success");
}
event => panic!("expected AuthCompleted event, got {event:?}"),
}
proxy.shutdown().await;
}
// --- Slack relay OAuth CSRF tests --- // --- Slack relay OAuth CSRF tests ---
fn test_relay_oauth_router(state: Arc<GatewayState>) -> Router { fn test_relay_oauth_router(state: Arc<GatewayState>) -> Router {
+5 -184
View File
@@ -473,8 +473,7 @@ pub struct PendingOAuthFlow {
pub secrets: Arc<dyn SecretsStore + Send + Sync>, pub secrets: Arc<dyn SecretsStore + Send + Sync>,
/// SSE broadcast manager for notifying the web UI. /// SSE broadcast manager for notifying the web UI.
pub sse_manager: Option<Arc<crate::channels::web::sse::SseManager>>, pub sse_manager: Option<Arc<crate::channels::web::sse::SseManager>>,
/// OAuth proxy auth token for authenticating with the hosted token exchange proxy. /// Gateway auth token for authenticating with the platform token exchange proxy.
/// Kept as `gateway_token` for public API compatibility.
pub gateway_token: Option<String>, pub gateway_token: Option<String>,
/// Additional form params for the token exchange request. /// Additional form params for the token exchange request.
/// Used for provider-specific requirements such as RFC 8707 `resource`. /// Used for provider-specific requirements such as RFC 8707 `resource`.
@@ -497,12 +496,6 @@ impl std::fmt::Debug for PendingOAuthFlow {
} }
} }
impl PendingOAuthFlow {
pub fn oauth_proxy_auth_token(&self) -> Option<&str> {
self.gateway_token.as_deref()
}
}
/// Thread-safe registry of pending OAuth flows, keyed by CSRF `state` parameter. /// Thread-safe registry of pending OAuth flows, keyed by CSRF `state` parameter.
pub type PendingOAuthRegistry = Arc<RwLock<HashMap<String, PendingOAuthFlow>>>; pub type PendingOAuthRegistry = Arc<RwLock<HashMap<String, PendingOAuthFlow>>>;
@@ -536,22 +529,6 @@ pub fn exchange_proxy_url() -> Option<String> {
.filter(|url| !url.is_empty()) .filter(|url| !url.is_empty())
} }
/// Returns the configured OAuth proxy auth token, if any.
///
/// New hosted infra can inject a dedicated shared proxy secret via
/// `IRONCLAW_OAUTH_PROXY_AUTH_TOKEN`. Existing hosted instances continue to
/// work by falling back to `GATEWAY_AUTH_TOKEN`.
pub fn oauth_proxy_auth_token() -> Option<String> {
fn normalized_env_value(key: &str) -> Option<String> {
crate::config::helpers::env_or_override(key)
.map(|value| value.trim().to_string())
.filter(|value| !value.is_empty())
}
normalized_env_value("IRONCLAW_OAUTH_PROXY_AUTH_TOKEN")
.or_else(|| normalized_env_value("GATEWAY_AUTH_TOKEN"))
}
/// Maximum age for pending OAuth flows (5 minutes, matching TCP listener timeout). /// Maximum age for pending OAuth flows (5 minutes, matching TCP listener timeout).
pub const OAUTH_FLOW_EXPIRY: Duration = Duration::from_secs(300); pub const OAUTH_FLOW_EXPIRY: Duration = Duration::from_secs(300);
@@ -697,8 +674,6 @@ pub fn strip_instance_prefix(state: &str) -> &str {
pub struct ProxyTokenExchangeRequest<'a> { pub struct ProxyTokenExchangeRequest<'a> {
pub proxy_url: &'a str, pub proxy_url: &'a str,
/// OAuth proxy auth token.
/// Kept as `gateway_token` for public API compatibility.
pub gateway_token: &'a str, pub gateway_token: &'a str,
pub token_url: &'a str, pub token_url: &'a str,
pub client_id: &'a str, pub client_id: &'a str,
@@ -712,8 +687,6 @@ pub struct ProxyTokenExchangeRequest<'a> {
pub struct ProxyRefreshTokenRequest<'a> { pub struct ProxyRefreshTokenRequest<'a> {
pub proxy_url: &'a str, pub proxy_url: &'a str,
/// OAuth proxy auth token.
/// Kept as `gateway_token` for public API compatibility.
pub gateway_token: &'a str, pub gateway_token: &'a str,
pub token_url: &'a str, pub token_url: &'a str,
pub client_id: &'a str, pub client_id: &'a str,
@@ -756,7 +729,7 @@ fn oauth_token_response_from_json(
/// Exchange an OAuth authorization code via the platform's token exchange proxy. /// Exchange an OAuth authorization code via the platform's token exchange proxy.
/// ///
/// Authenticated via an OAuth proxy auth token (Bearer header). The caller may /// Authenticated via the gateway auth token (Bearer header). The caller may
/// either rely on proxy-side secret lookup or forward a `client_secret` when /// either rely on proxy-side secret lookup or forward a `client_secret` when
/// the provider requires it. /// the provider requires it.
/// ///
@@ -768,7 +741,7 @@ pub async fn exchange_via_proxy(
) -> Result<OAuthTokenResponse, OAuthCallbackError> { ) -> Result<OAuthTokenResponse, OAuthCallbackError> {
if request.gateway_token.is_empty() { if request.gateway_token.is_empty() {
return Err(OAuthCallbackError::Io( return Err(OAuthCallbackError::Io(
"OAuth proxy auth token is required for proxy token exchange".to_string(), "Gateway auth token is required for proxy token exchange".to_string(),
)); ));
} }
let exchange_url = format!("{}/oauth/exchange", request.proxy_url.trim_end_matches('/')); let exchange_url = format!("{}/oauth/exchange", request.proxy_url.trim_end_matches('/'));
@@ -823,7 +796,7 @@ pub async fn exchange_via_proxy(
/// Refresh an OAuth access token via the platform's token refresh proxy. /// Refresh an OAuth access token via the platform's token refresh proxy.
/// ///
/// Authenticated via an OAuth proxy auth token (Bearer header). The caller may /// Authenticated via the gateway auth token (Bearer header). The caller may
/// either rely on proxy-side secret lookup or forward a `client_secret` when /// either rely on proxy-side secret lookup or forward a `client_secret` when
/// the provider requires it. /// the provider requires it.
pub async fn refresh_token_via_proxy( pub async fn refresh_token_via_proxy(
@@ -831,7 +804,7 @@ pub async fn refresh_token_via_proxy(
) -> Result<OAuthTokenResponse, OAuthCallbackError> { ) -> Result<OAuthTokenResponse, OAuthCallbackError> {
if request.gateway_token.is_empty() { if request.gateway_token.is_empty() {
return Err(OAuthCallbackError::Io( return Err(OAuthCallbackError::Io(
"OAuth proxy auth token is required for proxy token refresh".to_string(), "Gateway auth token is required for proxy token refresh".to_string(),
)); ));
} }
@@ -1037,37 +1010,6 @@ mod tests {
} }
} }
struct EnvVarGuard {
key: &'static str,
original: Option<String>,
}
impl Drop for EnvVarGuard {
fn drop(&mut self) {
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
if let Some(ref value) = self.original {
std::env::set_var(self.key, value);
} else {
std::env::remove_var(self.key);
}
}
}
}
fn set_env_var(key: &'static str, value: Option<&str>) -> EnvVarGuard {
let original = std::env::var(key).ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
if let Some(value) = value {
std::env::set_var(key, value);
} else {
std::env::remove_var(key);
}
}
EnvVarGuard { key, original }
}
#[test] #[test]
fn test_hosted_proxy_client_secret_suppresses_builtin_secret() { fn test_hosted_proxy_client_secret_suppresses_builtin_secret() {
let builtin = builtin_credentials("google_oauth_token").expect("google builtin creds"); let builtin = builtin_credentials("google_oauth_token").expect("google builtin creds");
@@ -1088,79 +1030,6 @@ mod tests {
assert_eq!(result, client_secret); assert_eq!(result, client_secret);
} }
#[tokio::test]
async fn test_exchange_via_proxy_sends_auth_and_form() {
let server = MockProxyServer::start().await;
let mut extra_token_params = HashMap::new();
extra_token_params.insert("resource".to_string(), "https://mcp.notion.com".to_string());
let response = super::exchange_via_proxy(super::ProxyTokenExchangeRequest {
proxy_url: &server.base_url(),
gateway_token: "shared-oauth-proxy-secret",
code: "auth-code-123",
redirect_uri: "https://oauth.example.com/oauth/callback",
token_url: "https://oauth2.googleapis.com/token",
client_id: TEST_OAUTH_CLIENT_ID,
client_secret: Some(TEST_OAUTH_CLIENT_SECRET),
access_token_field: "access_token",
code_verifier: Some("code-verifier-123"),
extra_token_params: &extra_token_params,
})
.await
.expect("proxy exchange succeeds");
assert_eq!(response.access_token, "proxy-access-token");
assert_eq!(
response.refresh_token.as_deref(),
Some("proxy-refresh-token")
);
assert_eq!(response.expires_in, Some(7200));
let requests = server.requests().await;
assert_eq!(requests.len(), 1);
assert_eq!(
requests[0].authorization.as_deref(),
Some("Bearer shared-oauth-proxy-secret")
);
assert_eq!(
requests[0].form.get("code").map(String::as_str),
Some("auth-code-123")
);
assert_eq!(
requests[0].form.get("redirect_uri").map(String::as_str),
Some("https://oauth.example.com/oauth/callback")
);
assert_eq!(
requests[0].form.get("token_url").map(String::as_str),
Some("https://oauth2.googleapis.com/token")
);
assert_eq!(
requests[0].form.get("client_id").map(String::as_str),
Some(TEST_OAUTH_CLIENT_ID)
);
assert_eq!(
requests[0].form.get("client_secret").map(String::as_str),
Some(TEST_OAUTH_CLIENT_SECRET)
);
assert_eq!(
requests[0]
.form
.get("access_token_field")
.map(String::as_str),
Some("access_token")
);
assert_eq!(
requests[0].form.get("code_verifier").map(String::as_str),
Some("code-verifier-123")
);
assert_eq!(
requests[0].form.get("resource").map(String::as_str),
Some("https://mcp.notion.com")
);
server.shutdown().await;
}
#[tokio::test] #[tokio::test]
async fn test_refresh_token_via_proxy_sends_auth_and_form() { async fn test_refresh_token_via_proxy_sends_auth_and_form() {
let server = MockProxyServer::start().await; let server = MockProxyServer::start().await;
@@ -1666,54 +1535,6 @@ mod tests {
} }
} }
#[test]
fn test_oauth_proxy_auth_token_prefers_dedicated_env() {
let _guard = lock_env();
let _proxy_guard = set_env_var(
"IRONCLAW_OAUTH_PROXY_AUTH_TOKEN",
Some("shared-proxy-secret"),
);
let _gateway_guard = set_env_var("GATEWAY_AUTH_TOKEN", Some("gateway-token"));
assert_eq!(
crate::cli::oauth_defaults::oauth_proxy_auth_token().as_deref(),
Some("shared-proxy-secret")
);
}
#[test]
fn test_oauth_proxy_auth_token_falls_back_to_gateway_token() {
let _guard = lock_env();
let _proxy_guard = set_env_var("IRONCLAW_OAUTH_PROXY_AUTH_TOKEN", None);
let _gateway_guard = set_env_var("GATEWAY_AUTH_TOKEN", Some("gateway-token"));
assert_eq!(
crate::cli::oauth_defaults::oauth_proxy_auth_token().as_deref(),
Some("gateway-token")
);
}
#[test]
fn test_oauth_proxy_auth_token_whitespace_dedicated_env_falls_back_to_gateway_token() {
let _guard = lock_env();
let _proxy_guard = set_env_var("IRONCLAW_OAUTH_PROXY_AUTH_TOKEN", Some(" "));
let _gateway_guard = set_env_var("GATEWAY_AUTH_TOKEN", Some("gateway-token"));
assert_eq!(
crate::cli::oauth_defaults::oauth_proxy_auth_token().as_deref(),
Some("gateway-token")
);
}
#[test]
fn test_oauth_proxy_auth_token_returns_none_when_unset() {
let _guard = lock_env();
let _proxy_guard = set_env_var("IRONCLAW_OAUTH_PROXY_AUTH_TOKEN", None);
let _gateway_guard = set_env_var("GATEWAY_AUTH_TOKEN", None);
assert_eq!(crate::cli::oauth_defaults::oauth_proxy_auth_token(), None);
}
#[test] #[test]
fn test_strip_instance_prefix_with_colon() { fn test_strip_instance_prefix_with_colon() {
use crate::cli::oauth_defaults::strip_instance_prefix; use crate::cli::oauth_defaults::strip_instance_prefix;
-3
View File
@@ -192,9 +192,6 @@ pub struct JobContext {
/// but subsequent tools (e.g., `json`) may need the full output. This /// but subsequent tools (e.g., `json`) may need the full output. This
/// stash stores the complete, unsanitized output so tools can reference /// stash stores the complete, unsanitized output so tools can reference
/// previous results by ID via `$tool_call_id` parameter syntax. /// previous results by ID via `$tool_call_id` parameter syntax.
///
/// Also used for cross-tool implicit state (keys prefixed with `__`) such
/// as `__routine_last_name` for fallback recovery in routine tool chains.
#[serde(skip)] #[serde(skip)]
pub tool_output_stash: Arc<tokio::sync::RwLock<HashMap<String, String>>>, pub tool_output_stash: Arc<tokio::sync::RwLock<HashMap<String, String>>>,
/// User's preferred timezone (IANA name, e.g. "America/New_York"). Defaults to "UTC". /// User's preferred timezone (IANA name, e.g. "America/New_York"). Defaults to "UTC".
+32 -392
View File
@@ -403,10 +403,9 @@ pub struct ExtensionManager {
/// when running in gateway mode, consumed by the web gateway's /// when running in gateway mode, consumed by the web gateway's
/// `/oauth/callback` handler. /// `/oauth/callback` handler.
pending_oauth_flows: crate::cli::oauth_defaults::PendingOAuthRegistry, pending_oauth_flows: crate::cli::oauth_defaults::PendingOAuthRegistry,
/// OAuth proxy auth token for authenticating with the hosted token exchange proxy. /// Gateway auth token for authenticating with the platform token exchange proxy.
/// Resolved once at construction from `IRONCLAW_OAUTH_PROXY_AUTH_TOKEN`, /// Read once at construction from `GATEWAY_AUTH_TOKEN` env var.
/// then `GATEWAY_AUTH_TOKEN` as a backward-compatible fallback. gateway_token: Option<String>,
oauth_proxy_auth_token: Option<String>,
/// Relay config captured at startup. Used by `auth_channel_relay` and /// Relay config captured at startup. Used by `auth_channel_relay` and
/// `activate_channel_relay` instead of re-reading env vars. /// `activate_channel_relay` instead of re-reading env vars.
relay_config: Option<crate::config::RelayConfig>, relay_config: Option<crate::config::RelayConfig>,
@@ -536,7 +535,7 @@ impl ExtensionManager {
activation_errors: RwLock::new(HashMap::new()), activation_errors: RwLock::new(HashMap::new()),
sse_manager: RwLock::new(None), sse_manager: RwLock::new(None),
pending_oauth_flows: crate::cli::oauth_defaults::new_pending_oauth_registry(), pending_oauth_flows: crate::cli::oauth_defaults::new_pending_oauth_registry(),
oauth_proxy_auth_token: crate::cli::oauth_defaults::oauth_proxy_auth_token(), gateway_token: std::env::var("GATEWAY_AUTH_TOKEN").ok(),
relay_config: crate::config::RelayConfig::from_env(), relay_config: crate::config::RelayConfig::from_env(),
relay_event_tx: Arc::new(tokio::sync::Mutex::new(None)), relay_event_tx: Arc::new(tokio::sync::Mutex::new(None)),
relay_signing_secret_cache: Arc::new(std::sync::Mutex::new(None)), relay_signing_secret_cache: Arc::new(std::sync::Mutex::new(None)),
@@ -660,66 +659,6 @@ impl ExtensionManager {
}) })
} }
/// Resolve the relay URL override for an extension from settings.
///
/// Returns `Some(url)` if a non-empty per-extension `relay_url` override is
/// set for the given extension; otherwise returns `None` and callers should
/// fall back to the env-level `RelayConfig`.
///
/// Uses `self.user_id` (owner scope) for consistency with `configure()`,
/// which also writes setting_path fields under the owner scope.
///
/// The override is validated: only `http` / `https` schemes are accepted
/// and the URL must not contain userinfo (embedded credentials). This
/// prevents a malicious override from exfiltrating the instance-wide relay
/// API key to an attacker-controlled host.
async fn effective_relay_url(&self, name: &str) -> Option<String> {
if let Some(ref store) = self.store {
let key = format!("extensions.{name}.relay_url");
if let Ok(Some(v)) = store.get_setting(&self.user_id, &key).await {
let url = v
.as_str()
.map(|s| s.trim().to_string())
.filter(|s| !s.is_empty());
if let Some(ref u) = url {
// Validate the override to prevent API-key exfiltration:
// only allow http(s) with no embedded credentials.
match url::Url::parse(u) {
Ok(parsed)
if (parsed.scheme() == "http" || parsed.scheme() == "https")
&& parsed.username().is_empty()
&& parsed.password().is_none() =>
{
tracing::debug!(
extension = %name,
relay_url_host = %parsed.host_str().unwrap_or("unknown"),
"effective_relay_url: using per-extension override from settings"
);
return url;
}
Ok(parsed) => {
tracing::warn!(
extension = %name,
scheme = %parsed.scheme(),
has_userinfo = !parsed.username().is_empty() || parsed.password().is_some(),
"effective_relay_url: rejecting override — \
only http/https without embedded credentials is allowed"
);
}
Err(e) => {
tracing::warn!(
extension = %name,
error = %e,
"effective_relay_url: rejecting override — invalid URL"
);
}
}
}
}
}
None
}
/// Get the shared relay event sender for the webhook endpoint. /// Get the shared relay event sender for the webhook endpoint.
pub fn relay_event_tx( pub fn relay_event_tx(
&self, &self,
@@ -953,46 +892,6 @@ impl ExtensionManager {
false false
} }
/// Check whether a stored `team_id` setting exists for the given relay extension.
///
/// Unlike [`is_relay_channel`], this does **not** consult the in-memory
/// `installed_relay_extensions` set — it only looks at the persistent settings
/// store. This distinction matters for `auth_channel_relay`: an extension can
/// be *installed* (present in the in-memory set) but not yet *authenticated*
/// (no OAuth completed, no team_id stored).
async fn has_stored_team_id(&self, name: &str, _user_id: &str) -> bool {
if let Some(ref store) = self.store {
let key = format!("relay:{}:team_id", name);
// Use owner scope (self.user_id) for consistency: the OAuth callback
// stores team_id under state.owner_id which maps to self.user_id.
match store.get_setting(&self.user_id, &key).await {
Ok(Some(v)) => {
let has_id = v.as_str().is_some_and(|s| !s.is_empty());
tracing::debug!(
extension = %name,
has_team_id = has_id,
"has_stored_team_id: checked store"
);
return has_id;
}
Ok(None) => {
tracing::debug!(
extension = %name,
"has_stored_team_id: no team_id setting found"
);
}
Err(e) => {
tracing::warn!(
extension = %name,
error = %e,
"has_stored_team_id: failed to read from settings store"
);
}
}
}
false
}
/// Restore persisted relay channels after startup. /// Restore persisted relay channels after startup.
/// ///
/// Loads the persisted active channel list, filters to relay types (those with /// Loads the persisted active channel list, filters to relay types (those with
@@ -1519,7 +1418,7 @@ impl ExtensionManager {
let errors = self.activation_errors.read().await; let errors = self.activation_errors.read().await;
for name in installed.iter() { for name in installed.iter() {
let active = active_names.contains(name); let active = active_names.contains(name);
let authenticated = self.has_stored_team_id(name, user_id).await; let authenticated = self.is_relay_channel(name, user_id).await;
let activation_error = errors.get(name).cloned(); let activation_error = errors.get(name).cloned();
let registry_entry = self let registry_entry = self
.registry .registry
@@ -2789,7 +2688,7 @@ impl ExtensionManager {
user_id: user_id.to_string(), user_id: user_id.to_string(),
secrets: Arc::clone(&self.secrets), secrets: Arc::clone(&self.secrets),
sse_manager: self.sse_manager.read().await.clone(), sse_manager: self.sse_manager.read().await.clone(),
gateway_token: self.oauth_proxy_auth_token.clone(), gateway_token: self.gateway_token.clone(),
token_exchange_extra_params, token_exchange_extra_params,
client_id_secret_name: if server.oauth.is_none() { client_id_secret_name: if server.oauth.is_none() {
Some(server.client_id_secret_name()) Some(server.client_id_secret_name())
@@ -3306,7 +3205,7 @@ impl ExtensionManager {
user_id: user_id.to_string(), user_id: user_id.to_string(),
secrets: Arc::clone(&self.secrets), secrets: Arc::clone(&self.secrets),
sse_manager: self.sse_manager.read().await.clone(), sse_manager: self.sse_manager.read().await.clone(),
gateway_token: self.oauth_proxy_auth_token.clone(), gateway_token: self.gateway_token.clone(),
token_exchange_extra_params: std::collections::HashMap::new(), token_exchange_extra_params: std::collections::HashMap::new(),
client_id_secret_name: None, client_id_secret_name: None,
created_at: std::time::Instant::now(), created_at: std::time::Instant::now(),
@@ -4292,69 +4191,20 @@ impl ExtensionManager {
name: &str, name: &str,
user_id: &str, user_id: &str,
) -> Result<AuthResult, ExtensionError> { ) -> Result<AuthResult, ExtensionError> {
tracing::debug!( // Check if already authenticated (team_id setting exists)
extension = %name, if self.is_relay_channel(name, user_id).await {
user_id = %user_id,
"auth_channel_relay: starting"
);
// Check if already authenticated by looking for a stored team_id.
// We intentionally skip the `installed_relay_extensions` in-memory set
// here because that set only tracks *installed* extensions — an extension
// can be installed (via registry) but not yet authenticated (no OAuth
// completed). Checking just `is_relay_channel()` would short-circuit
// to "authenticated" even when no team_id exists, preventing the OAuth
// flow from being offered to the user.
if self.has_stored_team_id(name, user_id).await {
tracing::debug!(
extension = %name,
"auth_channel_relay: already authenticated (team_id in store)"
);
return Ok(AuthResult::authenticated(name, ExtensionKind::ChannelRelay)); return Ok(AuthResult::authenticated(name, ExtensionKind::ChannelRelay));
} }
tracing::debug!(
extension = %name,
"auth_channel_relay: no stored team_id, initiating OAuth"
);
// Use relay config captured at startup // Use relay config captured at startup
let relay_config = self.relay_config().map_err(|e| { let relay_config = self.relay_config()?;
tracing::warn!(
extension = %name,
error = %e,
"auth_channel_relay: relay config not available — \
CHANNEL_RELAY_URL and CHANNEL_RELAY_API_KEY must be set"
);
e
})?;
// Allow per-extension URL override from settings
let effective_url = self
.effective_relay_url(name)
.await
.unwrap_or_else(|| relay_config.url.clone());
tracing::debug!(
extension = %name,
relay_url = %effective_url,
"auth_channel_relay: creating relay client for OAuth"
);
let client = crate::channels::relay::RelayClient::new( let client = crate::channels::relay::RelayClient::new(
effective_url.clone(), relay_config.url.clone(),
relay_config.api_key.clone(), relay_config.api_key.clone(),
relay_config.request_timeout_secs, relay_config.request_timeout_secs,
) )
.map_err(|e| { .map_err(|e| ExtensionError::Config(e.to_string()))?;
tracing::warn!(
extension = %name,
relay_url = %effective_url,
error = %e,
"auth_channel_relay: failed to create relay HTTP client"
);
ExtensionError::Config(e.to_string())
})?;
// Generate CSRF nonce — IronClaw validates this on the callback to ensure // Generate CSRF nonce — IronClaw validates this on the callback to ensure
// the OAuth completion is legitimate. Channel-relay embeds it in the signed // the OAuth completion is legitimate. Channel-relay embeds it in the signed
@@ -4366,44 +4216,18 @@ impl ExtensionManager {
self.secrets self.secrets
.create(user_id, CreateSecretParams::new(&state_key, &state_nonce)) .create(user_id, CreateSecretParams::new(&state_key, &state_nonce))
.await .await
.map_err(|e| { .map_err(|e| ExtensionError::AuthFailed(format!("Failed to store OAuth state: {e}")))?;
tracing::warn!(
extension = %name,
error = %e,
"auth_channel_relay: failed to store OAuth state nonce"
);
ExtensionError::AuthFailed(format!("Failed to store OAuth state: {e}"))
})?;
// Channel-relay derives all URLs from trusted instance_url in chat-api. // Channel-relay derives all URLs from trusted instance_url in chat-api.
// We only pass the nonce for CSRF validation on the callback. // We only pass the nonce for CSRF validation on the callback.
tracing::debug!(
extension = %name,
relay_url = %effective_url,
"auth_channel_relay: calling initiate_oauth on channel-relay"
);
match client.initiate_oauth(Some(&state_nonce)).await { match client.initiate_oauth(Some(&state_nonce)).await {
Ok(auth_url) => { Ok(auth_url) => Ok(AuthResult::awaiting_authorization(
tracing::info!(
extension = %name,
"auth_channel_relay: OAuth URL obtained, awaiting user authorization"
);
Ok(AuthResult::awaiting_authorization(
name, name,
ExtensionKind::ChannelRelay, ExtensionKind::ChannelRelay,
auth_url, auth_url,
"redirect".to_string(), "redirect".to_string(),
)) )),
} Err(e) => Err(ExtensionError::AuthFailed(e.to_string())),
Err(e) => {
tracing::warn!(
extension = %name,
relay_url = %effective_url,
error = %e,
"auth_channel_relay: initiate_oauth call to channel-relay failed"
);
Err(ExtensionError::AuthFailed(e.to_string()))
}
} }
} }
@@ -4413,112 +4237,40 @@ impl ExtensionManager {
name: &str, name: &str,
user_id: &str, user_id: &str,
) -> Result<ActivateResult, ExtensionError> { ) -> Result<ActivateResult, ExtensionError> {
tracing::debug!(
extension = %name,
user_id = %user_id,
"activate_channel_relay: starting"
);
let team_id_key = format!("relay:{}:team_id", name); let team_id_key = format!("relay:{}:team_id", name);
// Get team_id from settings (stored by the OAuth callback) // Get team_id from settings (stored by the OAuth callback)
let team_id = if let Some(ref store) = self.store { let team_id = if let Some(ref store) = self.store {
match store.get_setting(user_id, &team_id_key).await { store
Ok(Some(v)) => { .get_setting(user_id, &team_id_key)
let id = v.as_str().map(|s| s.to_string()).unwrap_or_default(); .await
tracing::debug!( .ok()
extension = %name, .flatten()
team_id_empty = id.is_empty(), .and_then(|v| v.as_str().map(|s| s.to_string()))
"activate_channel_relay: loaded team_id from store" .unwrap_or_default()
);
id
}
Ok(None) => {
tracing::debug!(
extension = %name,
setting_key = %team_id_key,
"activate_channel_relay: no team_id in settings store"
);
String::new()
}
Err(e) => {
tracing::warn!(
extension = %name,
error = %e,
"activate_channel_relay: failed to read team_id from settings store"
);
String::new()
}
}
} else { } else {
tracing::debug!(
extension = %name,
"activate_channel_relay: no settings store available"
);
String::new() String::new()
}; };
if team_id.is_empty() { if team_id.is_empty() {
tracing::debug!(
extension = %name,
"activate_channel_relay: team_id is empty, returning AuthRequired"
);
return Err(ExtensionError::AuthRequired); return Err(ExtensionError::AuthRequired);
} }
// Use relay config captured at startup // Use relay config captured at startup
let relay_config = self.relay_config().map_err(|e| { let relay_config = self.relay_config()?;
tracing::warn!(
extension = %name,
error = %e,
"activate_channel_relay: relay config not available"
);
e
})?;
// Allow per-extension URL override from settings
let effective_url = self
.effective_relay_url(name)
.await
.unwrap_or_else(|| relay_config.url.clone());
tracing::debug!(
extension = %name,
relay_url = %effective_url,
"activate_channel_relay: relay config loaded"
);
let instance_id = self.relay_instance_id(relay_config, user_id); let instance_id = self.relay_instance_id(relay_config, user_id);
let client = crate::channels::relay::RelayClient::new( let client = crate::channels::relay::RelayClient::new(
effective_url.clone(), relay_config.url.clone(),
relay_config.api_key.clone(), relay_config.api_key.clone(),
relay_config.request_timeout_secs, relay_config.request_timeout_secs,
) )
.map_err(|e| { .map_err(|e| ExtensionError::ActivationFailed(e.to_string()))?;
tracing::warn!(
extension = %name,
relay_url = %effective_url,
error = %e,
"activate_channel_relay: failed to create relay HTTP client"
);
ExtensionError::ActivationFailed(e.to_string())
})?;
// Fetch the per-instance signing secret from channel-relay. // Fetch the per-instance signing secret from channel-relay.
// This must succeed — there is no fallback. // This must succeed — there is no fallback.
tracing::debug!(
extension = %name,
relay_url = %effective_url,
"activate_channel_relay: fetching signing secret from channel-relay"
);
let signing_secret = client.get_signing_secret(&team_id).await.map_err(|e| { let signing_secret = client.get_signing_secret(&team_id).await.map_err(|e| {
tracing::warn!(
extension = %name,
relay_url = %effective_url,
error = %e,
"activate_channel_relay: failed to fetch signing secret from channel-relay"
);
ExtensionError::Config(format!("Failed to fetch relay signing secret: {e}")) ExtensionError::Config(format!("Failed to fetch relay signing secret: {e}"))
})?; })?;
@@ -4537,29 +4289,16 @@ impl ExtensionManager {
// Hot-add to channel manager // Hot-add to channel manager
let cm_guard = self.relay_channel_manager.read().await; let cm_guard = self.relay_channel_manager.read().await;
let channel_mgr = cm_guard.as_ref().ok_or_else(|| { let channel_mgr = cm_guard.as_ref().ok_or_else(|| {
tracing::warn!(
extension = %name,
"activate_channel_relay: channel manager not initialized"
);
ExtensionError::ActivationFailed("Channel manager not initialized".to_string()) ExtensionError::ActivationFailed("Channel manager not initialized".to_string())
})?; })?;
channel_mgr.hot_add(Box::new(channel)).await.map_err(|e| { channel_mgr
tracing::warn!( .hot_add(Box::new(channel))
extension = %name, .await
error = %e, .map_err(|e| ExtensionError::ActivationFailed(e.to_string()))?;
"activate_channel_relay: hot_add to channel manager failed"
);
ExtensionError::ActivationFailed(e.to_string())
})?;
if let Ok(mut cache) = self.relay_signing_secret_cache.lock() { if let Ok(mut cache) = self.relay_signing_secret_cache.lock() {
*cache = Some(signing_secret); *cache = Some(signing_secret);
} else {
tracing::warn!(
extension = %name,
"activate_channel_relay: failed to cache signing secret (mutex poisoned)"
);
} }
// Store the event sender so the web gateway's relay webhook endpoint can push events // Store the event sender so the web gateway's relay webhook endpoint can push events
@@ -4577,12 +4316,6 @@ impl ExtensionManager {
self.broadcast_extension_status(name, "active", Some(&status_msg)) self.broadcast_extension_status(name, "active", Some(&status_msg))
.await; .await;
tracing::info!(
extension = %name,
instance_id = %instance_id,
"activate_channel_relay: relay channel activated successfully"
);
Ok(ActivateResult { Ok(ActivateResult {
name: name.to_string(), name: name.to_string(),
kind: ExtensionKind::ChannelRelay, kind: ExtensionKind::ChannelRelay,
@@ -4862,41 +4595,6 @@ impl ExtensionManager {
} }
Ok(ExtensionSetupSchema { secrets, fields }) Ok(ExtensionSetupSchema { secrets, fields })
} }
ExtensionKind::ChannelRelay => {
let relay_url_key = format!("extensions.{name}.relay_url");
let current_url = if let Some(ref store) = self.store {
match store.get_setting(&self.user_id, &relay_url_key).await {
Ok(value_opt) => value_opt
.and_then(|v| v.as_str().map(|s| s.to_string()))
.filter(|s| !s.is_empty()),
Err(e) => {
tracing::warn!(
extension = %name,
setting_key = %relay_url_key,
error = %e,
"get_setup_schema: failed to read relay_url from settings"
);
None
}
}
} else {
None
};
let env_url = self.relay_config.as_ref().map(|c| c.url.as_str());
Ok(ExtensionSetupSchema {
secrets: Vec::new(),
fields: vec![crate::channels::web::types::SetupFieldInfo {
name: "relay_url".to_string(),
prompt: format!(
"Channel-relay service URL (leave empty to use env default{})",
env_url.map(|u| format!(": {u}")).unwrap_or_default()
),
optional: true,
provided: current_url.is_some(),
input_type: crate::tools::wasm::ToolSetupFieldInputType::Text,
}],
})
}
_ => Ok(ExtensionSetupSchema { _ => Ok(ExtensionSetupSchema {
secrets: Vec::new(), secrets: Vec::new(),
fields: Vec::new(), fields: Vec::new(),
@@ -5299,17 +4997,7 @@ impl ExtensionManager {
names.insert(server.token_secret_name()); names.insert(server.token_secret_name());
(names, Vec::new()) (names, Vec::new())
} }
ExtensionKind::ChannelRelay => { ExtensionKind::ChannelRelay => (std::collections::HashSet::new(), Vec::new()),
let relay_fields = vec![crate::tools::wasm::ToolFieldSetupSchema {
name: "relay_url".to_string(),
prompt: "Channel-relay service URL override".to_string(),
optional: true,
setting_path: Some(format!("extensions.{name}.relay_url")),
input_type: crate::tools::wasm::ToolSetupFieldInputType::Text,
restart_required: false,
}];
(std::collections::HashSet::new(), relay_fields)
}
}; };
let allowed_fields: std::collections::HashSet<String> = let allowed_fields: std::collections::HashSet<String> =
@@ -5400,28 +5088,13 @@ impl ExtensionManager {
))); )));
} }
let trimmed = field_value.trim(); let trimmed = field_value.trim();
let field_def = setup_field_defs.get(field_name);
// Empty value on an optional field with a setting_path: clear the
// stored override so the system reverts to the env/default value.
if trimmed.is_empty() { if trimmed.is_empty() {
if let Some(def) = field_def
&& def.optional
{
stored_fields.remove(field_name);
if let Some(setting_path) = &def.setting_path {
Self::validate_setup_setting_path(name, setting_path)?;
if let Some(store) = self.store.as_ref() {
let _ = store.delete_setting(&self.user_id, setting_path).await;
}
}
}
continue; continue;
} }
stored_fields.insert(field_name.clone(), trimmed.to_string()); stored_fields.insert(field_name.clone(), trimmed.to_string());
if let Some(field_def) = field_def { if let Some(field_def) = setup_field_defs.get(field_name) {
if field_def.restart_required { if field_def.restart_required {
restart_required = true; restart_required = true;
} }
@@ -7385,39 +7058,6 @@ mod tests {
); );
} }
/// Regression: installed-but-not-authenticated relay must NOT short-circuit
/// `auth_channel_relay()` to "authenticated". Previously, `auth_channel_relay`
/// called `is_relay_channel()` which checked the in-memory
/// `installed_relay_extensions` set; that returned `true` even when no team_id
/// existed in the store, so the OAuth URL was never offered.
#[tokio::test]
async fn test_auth_channel_relay_installed_without_team_id_is_not_authenticated() {
let dir = tempfile::tempdir().expect("temp dir");
let mgr = make_test_manager(None, dir.path().to_path_buf());
// Mark as installed (simulates clicking Install in the UI)
mgr.installed_relay_extensions
.write()
.await
.insert("slack-relay".to_string());
// Without a stored team_id, auth should NOT return authenticated.
// It should fail because relay config is missing (no CHANNEL_RELAY_URL),
// but the key assertion is that it does NOT return Ok(authenticated).
let result = mgr.auth_channel_relay("slack-relay", "test").await;
match result {
Ok(ref auth_result) if auth_result.is_authenticated() => {
panic!(
"auth_channel_relay returned authenticated for installed-but-no-team-id relay; \
expected either an OAuth URL or a config error"
);
}
_ => {
// Config error (no relay URL) or awaiting_authorization — both are correct
}
}
}
#[tokio::test] #[tokio::test]
async fn test_remove_relay_shuts_down_via_relay_channel_manager() { async fn test_remove_relay_shuts_down_via_relay_channel_manager() {
// Regression: remove() only checked channel_runtime for shutdown, missing // Regression: remove() only checked channel_runtime for shutdown, missing
+2 -1
View File
@@ -2,7 +2,8 @@
//! and activation of channels, tools, and MCP servers. //! and activation of channels, tools, and MCP servers.
//! //!
//! Extensions are the user-facing abstraction that unifies three runtime kinds: //! Extensions are the user-facing abstraction that unifies three runtime kinds:
//! - **Channels** (Telegram, Slack, Discord) — messaging integrations (WASM) //! - **Channels** (Telegram, Slack, Discord) — messaging platform connections
//! and conversation transports (WASM)
//! - **Tools** — sandboxed capabilities (WASM) //! - **Tools** — sandboxed capabilities (WASM)
//! - **MCP servers** — external API integrations via Model Context Protocol //! - **MCP servers** — external API integrations via Model Context Protocol
//! //!
+64 -6
View File
@@ -1017,8 +1017,11 @@ Example:
"\n\n## Extensions\n\ "\n\n## Extensions\n\
You can search, install, and activate extensions to add new capabilities:\n\ You can search, install, and activate extensions to add new capabilities:\n\
- **Channels** (Telegram, Slack, Discord) — messaging integrations. \ - **Channels** (Telegram, Slack, Discord) — connect messaging platforms so users can \
When users ask about connecting a messaging platform, search for it as a channel.\n\ talk to you there. When users ask about connecting a messaging platform, search for it \
as a channel. Channels are not separate send-message tools; use normal assistant output \
to reply in the current conversation, and use the `message` tool only for proactive, \
background, or cross-channel outbound sends.\n\
- **Tools** — sandboxed functions that extend your abilities.\n\ - **Tools** — sandboxed functions that extend your abilities.\n\
- **MCP servers** — external API integrations via the Model Context Protocol.\n\n\ - **MCP servers** — external API integrations via the Model Context Protocol.\n\n\
Use `tool_search` to find extensions by name. Refer to them by their kind \ Use `tool_search` to find extensions by name. Refer to them by their kind \
@@ -1059,15 +1062,20 @@ Example:
let message_tool_hint = "\ let message_tool_hint = "\
\n\n## Proactive Messaging\n\ \n\n## Proactive Messaging\n\
For ordinary replies in the current conversation, respond normally without calling `message`.\n\
Send messages via Signal, Telegram, Slack, or other connected channels:\n\ Send messages via Signal, Telegram, Slack, or other connected channels:\n\
- `content` (required): the message text\n\ - `content` (required): the message text\n\
- `attachments` (optional): array of file paths to send\n\ - `attachments` (optional): array of file paths to send\n\
- `channel` (optional): which channel to use (signal, telegram, slack, etc.)\n\ - `channel` (optional): which channel to use (signal, telegram, slack, etc.)\n\
- `target` (optional): who to send to (phone number, group ID, etc.)\n\ - `target` (optional): who to send to (phone number, group ID, etc.)\n\
\nOmit both `channel` and `target` to send to the current conversation.\n\ \nOmit both `channel` and `target` for a proactive follow-up in the current conversation.\n\
Target formats:\n\
- Signal: E.164 phone number (`+1234567890`) or group ID\n\
- Telegram: username or chat ID\n\
- Slack: channel name (`#general`) or user ID\n\
Examples (tool calls use JSON format):\n\ Examples (tool calls use JSON format):\n\
- Reply here: {\"content\": \"Hi!\"}\n\ - Proactive follow-up here: {\"content\": \"Hi again!\"}\n\
- Send file here: {\"content\": \"Here's the file\", \"attachments\": [\"/path/to/file.txt\"]}\n\ - Send file here proactively: {\"content\": \"Here's the file\", \"attachments\": [\"/path/to/file.txt\"]}\n\
- Message a different user: {\"channel\": \"signal\", \"target\": \"+1234567890\", \"content\": \"Hi!\"}\n\ - Message a different user: {\"channel\": \"signal\", \"target\": \"+1234567890\", \"content\": \"Hi!\"}\n\
- Message a different group: {\"channel\": \"signal\", \"target\": \"group:abc123\", \"content\": \"Hi!\"}"; - Message a different group: {\"channel\": \"signal\", \"target\": \"group:abc123\", \"content\": \"Hi!\"}";
@@ -1105,7 +1113,9 @@ Examples (tool calls use JSON format):\n\
format!( format!(
"\n\n## Current Conversation\n\ "\n\n## Current Conversation\n\
This is who you're talking to (omit 'target' to send here):\n{}", This is who you're talking to in the active conversation. Use normal assistant \
output to reply here; only use the `message` tool for proactive, background, or \
cross-channel outbound sends:\n{}",
lines.join("\n") lines.join("\n")
) )
} }
@@ -2452,6 +2462,54 @@ That's my plan."#;
); );
} }
#[test]
fn test_extensions_section_clarifies_channels_are_not_send_tools() {
let reasoning = make_test_reasoning();
let tool_defs = vec![ToolDefinition {
name: "tool_search".to_string(),
description: "Search extensions".to_string(),
parameters: serde_json::json!({}),
}];
let section = reasoning.build_extensions_section_for_tools(&tool_defs);
assert!(section.contains("connect messaging platforms so users can talk to you there"));
assert!(section.contains("Channels are not separate send-message tools"));
assert!(
section.contains("use normal assistant output to reply in the current conversation")
);
assert!(section.contains(
"`message` tool only for proactive, background, or cross-channel outbound sends"
));
}
#[test]
fn test_channel_section_separates_normal_replies_from_message_tool() {
let reasoning = make_test_reasoning().with_channel("telegram");
let section = reasoning.build_channel_section();
assert!(section.contains("respond normally without calling `message`"));
assert!(section.contains("proactive follow-up in the current conversation"));
assert!(section.contains("Target formats:"));
assert!(section.contains("Signal: E.164 phone number"));
assert!(section.contains("Telegram: username or chat ID"));
assert!(section.contains("Slack: channel name"));
assert!(section.contains("Proactive follow-up here"));
}
#[test]
fn test_current_conversation_section_does_not_imply_message_tool_for_replies() {
let reasoning = make_test_reasoning()
.with_channel("telegram")
.with_conversation_data("User", "telegram-user");
let section = reasoning.build_conversation_section();
assert!(section.contains("Use normal assistant output to reply here"));
assert!(section.contains(
"only use the `message` tool for proactive, background, or cross-channel outbound sends"
));
assert!(!section.contains("omit 'target' to send here"));
}
// ---- plan/evaluate bypass clean_response (Bug #564-2) ---- // ---- plan/evaluate bypass clean_response (Bug #564-2) ----
#[test] #[test]
+16 -2
View File
@@ -31,8 +31,11 @@ impl Tool for ToolSearchTool {
fn description(&self) -> &str { fn description(&self) -> &str {
"Search for available extensions to add new capabilities. Extensions include \ "Search for available extensions to add new capabilities. Extensions include \
channels (Telegram, Slack, Discord — for messaging), tools, and MCP servers. \ channels (Telegram, Slack, Discord — connect messaging platforms so IronClaw can \
Use discover:true to search online if the built-in registry has no results." receive and reply there), tools, and MCP servers. Use `tool_install` and \
`tool_activate` to install and enable channels; use the `message` tool for proactive \
outbound sends. Use discover:true to search online if the built-in registry has no \
results."
} }
fn parameters_schema(&self) -> serde_json::Value { fn parameters_schema(&self) -> serde_json::Value {
@@ -634,6 +637,17 @@ mod tests {
assert!(schema["properties"].get("query").is_some()); assert!(schema["properties"].get("query").is_some());
} }
#[test]
fn test_tool_search_description_clarifies_channel_setup_vs_sending() {
let tool = ToolSearchTool {
manager: test_manager_stub(),
};
let description = tool.description();
assert!(description.contains("Use `tool_install` and `tool_activate`"));
assert!(description.contains("use the `message` tool for proactive outbound sends"));
}
#[test] #[test]
fn test_tool_install_schema() { fn test_tool_install_schema() {
use crate::tools::tool::ApprovalRequirement; use crate::tools::tool::ApprovalRequirement;
+13 -5
View File
@@ -181,11 +181,15 @@ impl Tool for MessageTool {
} }
fn description(&self) -> &str { fn description(&self) -> &str {
"Send a message to a channel. If channel/target omitted, uses the current conversation's \ "Send a proactive message to a channel. Use normal assistant output to reply in the \
channel and sender/group. Use to proactively message users on any connected channel. \ active conversation; use this tool for proactive notifications, routine/background \
follow-ups, attachments, or sending to a different channel/recipient. If channel/target \
are omitted, reuses the current conversation's channel and sender/group when available. \
If you provide `target` without `channel` and no scoped channel can be resolved, the \
message may be broadcast across connected channels instead of sent to just one. \
Supports file attachments: first download the file with the http tool using save_to \ Supports file attachments: first download the file with the http tool using save_to \
(e.g., http GET https://picsum.photos/800/600 save_to=/tmp/photo.jpg), then pass \ (e.g., http GET https://picsum.photos/800/600 save_to=/tmp/photo.jpg), then pass the \
the file path in the attachments array. Images are sent as photos on Telegram. \ file path in the attachments array. Images are sent as photos on Telegram. \
- Signal: target accepts E.164 (+1234567890) or group ID \ - Signal: target accepts E.164 (+1234567890) or group ID \
- Telegram: target accepts username or chat ID \ - Telegram: target accepts username or chat ID \
- Slack: target accepts channel (#general) or user ID" - Slack: target accepts channel (#general) or user ID"
@@ -451,7 +455,11 @@ mod tests {
#[test] #[test]
fn message_tool_description() { fn message_tool_description() {
let tool = MessageTool::new(Arc::new(ChannelManager::new())); let tool = MessageTool::new(Arc::new(ChannelManager::new()));
assert!(!tool.description().is_empty()); let description = tool.description();
assert!(!description.is_empty());
assert!(description.contains("Use normal assistant output to reply"));
assert!(description.contains("proactive notifications"));
assert!(description.contains("provide `target` without `channel`"));
} }
#[test] #[test]
+3 -35
View File
@@ -650,23 +650,6 @@ pub(crate) fn routine_update_parameters_schema() -> Value {
}) })
} }
const ROUTINE_LAST_NAME_STASH_KEY: &str = "__routine_last_name";
async fn stash_last_routine_name(ctx: &JobContext, name: &str) {
ctx.tool_output_stash
.write()
.await
.insert(ROUTINE_LAST_NAME_STASH_KEY.to_string(), name.to_string());
}
async fn restore_last_routine_name(ctx: &JobContext) -> Option<String> {
ctx.tool_output_stash
.read()
.await
.get(ROUTINE_LAST_NAME_STASH_KEY)
.cloned()
}
fn nested_object<'a>(params: &'a Value, field: &str) -> Option<&'a Map<String, Value>> { fn nested_object<'a>(params: &'a Value, field: &str) -> Option<&'a Map<String, Value>> {
params.get(field).and_then(Value::as_object) params.get(field).and_then(Value::as_object)
} }
@@ -1110,7 +1093,6 @@ impl Tool for RoutineCreateTool {
) -> Result<ToolOutput, ToolError> { ) -> Result<ToolOutput, ToolError> {
let start = std::time::Instant::now(); let start = std::time::Instant::now();
let normalized = parse_routine_create_request(&params)?; let normalized = parse_routine_create_request(&params)?;
stash_last_routine_name(ctx, &normalized.name).await;
let trigger = build_routine_trigger(&normalized.trigger); let trigger = build_routine_trigger(&normalized.trigger);
let action = let action =
build_routine_action(&normalized.name, &normalized.prompt, &normalized.execution); build_routine_action(&normalized.name, &normalized.prompt, &normalized.execution);
@@ -1292,7 +1274,6 @@ impl Tool for RoutineUpdateTool {
let start = std::time::Instant::now(); let start = std::time::Instant::now();
let name = require_str(&params, "name")?; let name = require_str(&params, "name")?;
stash_last_routine_name(ctx, name).await;
let mut routine = self let mut routine = self
.store .store
@@ -1430,24 +1411,11 @@ impl Tool for RoutineDeleteTool {
) -> Result<ToolOutput, ToolError> { ) -> Result<ToolOutput, ToolError> {
let start = std::time::Instant::now(); let start = std::time::Instant::now();
let name = if let Some(name) = params.get("name").and_then(|v| v.as_str()) { let name = require_str(&params, "name")?;
if name.trim().is_empty() {
return Err(ToolError::InvalidParameters(
"'name' parameter cannot be empty".to_string(),
));
}
name.to_string()
} else {
restore_last_routine_name(ctx).await.ok_or_else(|| {
ToolError::InvalidParameters(
"missing 'name' parameter and no previous routine target to infer".to_string(),
)
})?
};
let routine = self let routine = self
.store .store
.get_routine_by_name(&ctx.user_id, &name) .get_routine_by_name(&ctx.user_id, name)
.await .await
.map_err(|e| ToolError::ExecutionFailed(format!("DB error: {e}")))? .map_err(|e| ToolError::ExecutionFailed(format!("DB error: {e}")))?
.ok_or_else(|| ToolError::ExecutionFailed(format!("routine '{}' not found", name)))?; .ok_or_else(|| ToolError::ExecutionFailed(format!("routine '{}' not found", name)))?;
@@ -1462,7 +1430,7 @@ impl Tool for RoutineDeleteTool {
self.engine.refresh_event_cache().await; self.engine.refresh_event_cache().await;
let result = serde_json::json!({ let result = serde_json::json!({
"name": &name, "name": name,
"deleted": deleted, "deleted": deleted,
}); });
+1 -19
View File
@@ -117,11 +117,6 @@ impl McpClient {
/// The config must use HTTP transport (the default); for stdio/UDS use `new_with_transport`. /// 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. /// Returns an error if the config uses a non-HTTP transport.
///
/// **Note:** The session manager is NOT wired into the transport. For
/// production use, prefer `create_client_from_config()` which constructs
/// the transport with session tracking.
#[cfg(test)]
pub fn new_with_config(config: McpServerConfig) -> Result<Self, ToolError> { pub fn new_with_config(config: McpServerConfig) -> Result<Self, ToolError> {
if !matches!( if !matches!(
config.effective_transport(), config.effective_transport(),
@@ -219,14 +214,7 @@ impl McpClient {
} }
} }
/// Attach a session manager to the **client** only. /// Attach a session manager for Streamable HTTP session tracking.
///
/// **Warning:** This does NOT wire the session manager into the underlying
/// `HttpMcpTransport`, so the transport will not capture `Mcp-Session-Id`
/// from responses. For production use, construct the transport with
/// `HttpMcpTransport::with_session_manager()` and pass it to
/// `new_with_transport()` instead. See `create_client_from_config()`.
#[cfg(test)]
pub fn with_session_manager(mut self, session_manager: Arc<McpSessionManager>) -> Self { pub fn with_session_manager(mut self, session_manager: Arc<McpSessionManager>) -> Self {
self.session_manager = Some(session_manager); self.session_manager = Some(session_manager);
self self
@@ -247,12 +235,6 @@ impl McpClient {
self.session_manager.is_some() self.session_manager.is_some()
} }
/// Get the underlying transport (test-only).
#[cfg(test)]
pub(crate) fn transport(&self) -> &Arc<dyn McpTransport> {
&self.transport
}
/// Get the next request ID. /// Get the next request ID.
fn next_request_id(&self) -> u64 { fn next_request_id(&self) -> u64 {
self.next_id.fetch_add(1, Ordering::SeqCst) self.next_id.fetch_add(1, Ordering::SeqCst)
+17 -102
View File
@@ -7,7 +7,6 @@ use std::sync::Arc;
use crate::secrets::SecretsStore; use crate::secrets::SecretsStore;
use crate::tools::mcp::config::{EffectiveTransport, McpServerConfig}; use crate::tools::mcp::config::{EffectiveTransport, McpServerConfig};
use crate::tools::mcp::http_transport::HttpMcpTransport;
use crate::tools::mcp::{McpClient, McpProcessManager, McpSessionManager, McpTransport}; use crate::tools::mcp::{McpClient, McpProcessManager, McpSessionManager, McpTransport};
/// Error returned when MCP client creation fails. /// Error returned when MCP client creation fails.
@@ -79,37 +78,33 @@ pub async fn create_client_from_config(
Err(McpFactoryError::UnixNotSupported { name: server_name }) Err(McpFactoryError::UnixNotSupported { name: server_name })
} }
EffectiveTransport::Http => { EffectiveTransport::Http => {
// Authenticated (OAuth) path: tokens exist or server requires auth.
if let Some(ref secrets) = secrets { if let Some(ref secrets) = secrets {
let has_tokens = let has_tokens =
crate::tools::mcp::is_authenticated(&server, secrets, user_id).await; crate::tools::mcp::is_authenticated(&server, secrets, user_id).await;
if has_tokens || server.requires_auth() { if has_tokens || server.requires_auth() {
return Ok(McpClient::new_authenticated( Ok(McpClient::new_authenticated(
server, server,
Arc::clone(session_manager), Arc::clone(session_manager),
Arc::clone(secrets), Arc::clone(secrets),
user_id, user_id,
));
}
}
// Non-OAuth HTTP: wire the session manager into the *transport* so
// it captures `Mcp-Session-Id` from responses. Passing it only to
// the client (via `with_session_manager`) is not enough — the
// transport must know about it to read/write the header.
let transport = Arc::new(
HttpMcpTransport::new(server.url.clone(), server.name.clone())
.with_session_manager(Arc::clone(session_manager)),
);
Ok(McpClient::new_with_transport(
server.name.clone(),
transport,
Some(Arc::clone(session_manager)),
secrets,
user_id,
Some(server),
)) ))
} 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,84 +134,4 @@ mod tests {
"non-OAuth HTTP clients must carry a session manager" "non-OAuth HTTP clients must carry a session manager"
); );
} }
/// Regression test: the factory must wire the session manager into the
/// *transport*, not just the client. Otherwise the transport never
/// captures `Mcp-Session-Id` from responses and subsequent requests
/// lack the header, causing the server to reject them.
#[tokio::test]
async fn test_factory_non_oauth_http_transport_captures_session_id() {
use axum::http::header::HeaderName;
use axum::{Router, http::StatusCode, response::IntoResponse, routing::post};
use tokio::net::TcpListener;
const SESSION_ID: &str = "test-session-abc123";
async fn session_echo() -> impl IntoResponse {
let body = serde_json::json!({
"jsonrpc": "2.0",
"id": 1,
"result": {}
})
.to_string();
(
StatusCode::OK,
[(
HeaderName::from_static("mcp-session-id"),
SESSION_ID.to_string(),
)],
body,
)
}
let app = Router::new().route("/", post(session_echo));
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let url = format!("http://127.0.0.1:{}", addr.port());
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let server = McpServerConfig::new("session-test", &url);
let session_manager = Arc::new(McpSessionManager::new());
let process_manager = Arc::new(McpProcessManager::new());
let client = create_client_from_config(
server,
&session_manager,
&process_manager,
None,
"test-user",
)
.await
.expect("factory should succeed for HTTP config");
// Pre-create a session entry so that update_session_id has something to update.
// In production, the MCP initialize handshake calls get_or_create before responses arrive.
session_manager.get_or_create("session-test", &url).await;
// Send a request through the client's transport to trigger session capture.
use crate::tools::mcp::protocol::McpRequest;
let request = McpRequest {
jsonrpc: "2.0".to_string(),
id: Some(1),
method: "test".to_string(),
params: Some(serde_json::json!({})),
};
let headers = std::collections::HashMap::new();
client
.transport()
.send(&request, &headers)
.await
.expect("request should succeed");
// Verify the session manager captured the session ID from the response.
let captured = session_manager.get_session_id("session-test").await;
assert_eq!(
captured.as_deref(),
Some(SESSION_ID),
"transport must capture Mcp-Session-Id into session manager"
);
}
} }
-28
View File
@@ -494,34 +494,6 @@ mod tests {
assert_eq!(echoed["authorization"], "Bearer oauth-token"); assert_eq!(echoed["authorization"], "Bearer oauth-token");
} }
/// Regression test for #1436: 202 Accepted responses for notifications
/// were parsed as JSON, causing "Failed to parse MCP response" errors
/// that broke the MCP session handshake.
#[tokio::test]
async fn test_wire_202_accepted_for_notification() {
use axum::{Router, http::StatusCode, routing::post};
use tokio::net::TcpListener;
async fn accept_notification() -> StatusCode {
StatusCode::ACCEPTED
}
let app = Router::new().route("/", post(accept_notification));
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let url = format!("http://127.0.0.1:{}", addr.port());
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let transport = HttpMcpTransport::new(&url, "test-202");
let request = McpRequest::initialized_notification();
let response = transport.send(&request, &HashMap::new()).await.unwrap();
assert!(response.result.is_none());
assert!(response.error.is_none());
}
#[tokio::test] #[tokio::test]
async fn test_wire_custom_auth_preserved_when_no_per_request_auth() { async fn test_wire_custom_auth_preserved_when_no_per_request_auth() {
let (url, _handle) = spawn_echo_server().await; let (url, _handle) = spawn_echo_server().await;
+4 -51
View File
@@ -446,14 +446,16 @@ fn resolve_oauth_refresh_config(cap_file: &CapabilitiesFile) -> Option<OAuthRefr
builtin.as_ref(), builtin.as_ref(),
exchange_proxy_url.is_some(), exchange_proxy_url.is_some(),
); );
let oauth_proxy_auth_token = crate::cli::oauth_defaults::oauth_proxy_auth_token(); let gateway_token = crate::config::helpers::env_or_override("GATEWAY_AUTH_TOKEN")
.map(|token| token.trim().to_string())
.filter(|token| !token.is_empty());
Some(OAuthRefreshConfig { Some(OAuthRefreshConfig {
token_url: oauth.token_url.clone(), token_url: oauth.token_url.clone(),
client_id, client_id,
client_secret, client_secret,
exchange_proxy_url, exchange_proxy_url,
gateway_token: oauth_proxy_auth_token, gateway_token,
secret_name: auth.secret_name.clone(), secret_name: auth.secret_name.clone(),
provider: auth.provider.clone(), provider: auth.provider.clone(),
}) })
@@ -889,11 +891,6 @@ mod tests {
AuthCapabilitySchema, CapabilitiesFile, OAuthConfigSchema, AuthCapabilitySchema, CapabilitiesFile, OAuthConfigSchema,
}; };
let _guard = lock_env();
let _proxy_guard = set_env_var("IRONCLAW_OAUTH_EXCHANGE_URL", None);
let _gateway_token_guard = set_env_var("GATEWAY_AUTH_TOKEN", None);
let _oauth_proxy_token_guard = set_env_var("IRONCLAW_OAUTH_PROXY_AUTH_TOKEN", None);
let caps = CapabilitiesFile { let caps = CapabilitiesFile {
auth: Some(AuthCapabilitySchema { auth: Some(AuthCapabilitySchema {
secret_name: "google_oauth_token".to_string(), secret_name: "google_oauth_token".to_string(),
@@ -985,7 +982,6 @@ mod tests {
let _guard = lock_env(); let _guard = lock_env();
let _proxy_guard = set_env_var("IRONCLAW_OAUTH_EXCHANGE_URL", None); let _proxy_guard = set_env_var("IRONCLAW_OAUTH_EXCHANGE_URL", None);
let _gateway_token_guard = set_env_var("GATEWAY_AUTH_TOKEN", None); let _gateway_token_guard = set_env_var("GATEWAY_AUTH_TOKEN", None);
let _oauth_proxy_token_guard = set_env_var("IRONCLAW_OAUTH_PROXY_AUTH_TOKEN", None);
// google_oauth_token should fall back to built-in credentials // google_oauth_token should fall back to built-in credentials
let caps = CapabilitiesFile { let caps = CapabilitiesFile {
@@ -1025,7 +1021,6 @@ mod tests {
Some("https://compose-api.example.com"), Some("https://compose-api.example.com"),
); );
let _gateway_token_guard = set_env_var("GATEWAY_AUTH_TOKEN", Some("gateway-test-token")); let _gateway_token_guard = set_env_var("GATEWAY_AUTH_TOKEN", Some("gateway-test-token"));
let _oauth_proxy_token_guard = set_env_var("IRONCLAW_OAUTH_PROXY_AUTH_TOKEN", None);
let _client_id_guard = let _client_id_guard =
set_env_var("GOOGLE_OAUTH_CLIENT_ID", Some("hosted-google-client-id")); set_env_var("GOOGLE_OAUTH_CLIENT_ID", Some("hosted-google-client-id"));
@@ -1066,7 +1061,6 @@ mod tests {
Some("https://compose-api.example.com"), Some("https://compose-api.example.com"),
); );
let _gateway_token_guard = set_env_var("GATEWAY_AUTH_TOKEN", Some("gateway-test-token")); let _gateway_token_guard = set_env_var("GATEWAY_AUTH_TOKEN", Some("gateway-test-token"));
let _oauth_proxy_token_guard = set_env_var("IRONCLAW_OAUTH_PROXY_AUTH_TOKEN", None);
let _client_id_guard = let _client_id_guard =
set_env_var("GOOGLE_OAUTH_CLIENT_ID", Some("hosted-google-client-id")); set_env_var("GOOGLE_OAUTH_CLIENT_ID", Some("hosted-google-client-id"));
let _client_secret_guard = let _client_secret_guard =
@@ -1101,47 +1095,6 @@ mod tests {
assert_eq!(config.gateway_token.as_deref(), Some("gateway-test-token")); assert_eq!(config.gateway_token.as_deref(), Some("gateway-test-token"));
} }
#[test]
fn test_resolve_oauth_refresh_config_hosted_proxy_prefers_dedicated_proxy_auth_token() {
use crate::tools::wasm::capabilities_schema::{
AuthCapabilitySchema, CapabilitiesFile, OAuthConfigSchema,
};
let _guard = lock_env();
let _proxy_guard = set_env_var(
"IRONCLAW_OAUTH_EXCHANGE_URL",
Some("https://compose-api.example.com"),
);
let _gateway_token_guard = set_env_var("GATEWAY_AUTH_TOKEN", Some("gateway-test-token"));
let _oauth_proxy_token_guard = set_env_var(
"IRONCLAW_OAUTH_PROXY_AUTH_TOKEN",
Some("shared-oauth-proxy-secret"),
);
let _client_id_guard =
set_env_var("GOOGLE_OAUTH_CLIENT_ID", Some("hosted-google-client-id"));
let caps = CapabilitiesFile {
auth: Some(AuthCapabilitySchema {
secret_name: "google_oauth_token".to_string(),
provider: Some("google".to_string()),
oauth: Some(OAuthConfigSchema {
authorization_url: "https://accounts.google.com/o/oauth2/v2/auth".to_string(),
token_url: "https://oauth2.googleapis.com/token".to_string(),
client_id_env: Some("GOOGLE_OAUTH_CLIENT_ID".to_string()),
..Default::default()
}),
..Default::default()
}),
..Default::default()
};
let config = super::resolve_oauth_refresh_config(&caps).expect("hosted oauth config");
assert_eq!(
config.gateway_token.as_deref(),
Some("shared-oauth-proxy-secret")
);
}
// --------------------------------------------------------------- // ---------------------------------------------------------------
// Security regression tests // Security regression tests
// --------------------------------------------------------------- // ---------------------------------------------------------------
+5 -13
View File
@@ -62,8 +62,7 @@ pub struct OAuthRefreshConfig {
pub client_secret: Option<String>, pub client_secret: Option<String>,
/// Hosted OAuth proxy base URL (e.g., "http://host.docker.internal:8080"). /// Hosted OAuth proxy base URL (e.g., "http://host.docker.internal:8080").
pub exchange_proxy_url: Option<String>, pub exchange_proxy_url: Option<String>,
/// OAuth proxy auth token for authenticating with the hosted OAuth proxy. /// Gateway auth token for authenticating with the hosted OAuth proxy.
/// Kept as `gateway_token` for public API compatibility.
pub gateway_token: Option<String>, pub gateway_token: Option<String>,
/// Secret name of the access token (e.g., "google_oauth_token"). /// Secret name of the access token (e.g., "google_oauth_token").
/// The refresh token lives at `{secret_name}_refresh_token`. /// The refresh token lives at `{secret_name}_refresh_token`.
@@ -72,12 +71,6 @@ pub struct OAuthRefreshConfig {
pub provider: Option<String>, pub provider: Option<String>,
} }
impl OAuthRefreshConfig {
fn oauth_proxy_auth_token(&self) -> Option<&str> {
self.gateway_token.as_deref()
}
}
/// Pre-resolved credential for host-based injection. /// Pre-resolved credential for host-based injection.
/// ///
/// Built before each WASM execution by decrypting secrets from the store. /// Built before each WASM execution by decrypting secrets from the store.
@@ -1225,9 +1218,9 @@ async fn refresh_oauth_token(
let refresh_name = format!("{}_refresh_token", config.secret_name); let refresh_name = format!("{}_refresh_token", config.secret_name);
if let Some(proxy_url) = config.exchange_proxy_url.as_deref() { if let Some(proxy_url) = config.exchange_proxy_url.as_deref() {
let Some(oauth_proxy_auth_token) = config.oauth_proxy_auth_token() else { let Some(gateway_token) = config.gateway_token.as_deref() else {
tracing::warn!( tracing::warn!(
"OAuth refresh proxy is configured, but no OAuth proxy auth token is available" "OAuth refresh proxy is configured, but no gateway auth token is available"
); );
return false; return false;
}; };
@@ -1242,7 +1235,7 @@ async fn refresh_oauth_token(
let token_response = match oauth_defaults::refresh_token_via_proxy( let token_response = match oauth_defaults::refresh_token_via_proxy(
oauth_defaults::ProxyRefreshTokenRequest { oauth_defaults::ProxyRefreshTokenRequest {
proxy_url, proxy_url,
gateway_token: oauth_proxy_auth_token, gateway_token,
token_url: &config.token_url, token_url: &config.token_url,
client_id: &config.client_id, client_id: &config.client_id,
client_secret: config.client_secret.as_deref(), client_secret: config.client_secret.as_deref(),
@@ -2711,8 +2704,7 @@ mod tests {
} }
#[tokio::test] #[tokio::test]
async fn test_resolve_host_credentials_skips_refresh_token_lookup_without_oauth_proxy_auth_token() async fn test_resolve_host_credentials_skips_refresh_token_lookup_without_gateway_token() {
{
use crate::secrets::{ use crate::secrets::{
CreateSecretParams, CredentialLocation, CredentialMapping, SecretsStore, CreateSecretParams, CredentialLocation, CredentialMapping, SecretsStore,
}; };
+112 -41
View File
@@ -13,7 +13,7 @@ mod tests {
use ironclaw::agent::routine::{RoutineAction, Trigger}; use ironclaw::agent::routine::{RoutineAction, Trigger};
use crate::support::test_rig::TestRigBuilder; use crate::support::test_rig::TestRigBuilder;
use crate::support::trace_llm::LlmTrace; use crate::support::trace_llm::{LlmTrace, TraceResponse, TraceStep, TraceToolCall, TraceTurn};
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// Test 1: time_parse_and_diff // Test 1: time_parse_and_diff
@@ -205,44 +205,7 @@ mod tests {
} }
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// Test 5: routine_update_fail_delete_fallback // Test 5: routine_manual_create_defaults_to_tools_enabled
// -----------------------------------------------------------------------
#[tokio::test]
async fn routine_update_fail_delete_fallback() {
let trace = LlmTrace::from_file(concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/fixtures/llm_traces/tools/routine_update_fail_delete_fallback.json"
))
.expect("failed to load routine_update_fail_delete_fallback.json");
let rig = TestRigBuilder::new()
.with_trace(trace.clone())
.with_auto_approve_tools(true)
.build()
.await;
rig.send_message("Try converting a routine trigger, then recover by deleting it")
.await;
let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await;
rig.verify_trace_expects(&trace, &responses);
let completed = rig.tool_calls_completed();
assert!(
completed.iter().any(|(n, ok)| n == "routine_update" && !ok),
"routine_update should fail in this regression path: {completed:?}"
);
assert!(
completed.iter().any(|(n, ok)| n == "routine_delete" && *ok),
"routine_delete should recover successfully via preserved routine identity: {completed:?}"
);
rig.shutdown();
}
// -----------------------------------------------------------------------
// Test 6: routine_manual_create_defaults_to_tools_enabled
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
#[tokio::test] #[tokio::test]
@@ -283,7 +246,7 @@ mod tests {
} }
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// Test 7: routine_manual_create_explicit_no_tools // Test 6: routine_manual_create_explicit_no_tools
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
#[tokio::test] #[tokio::test]
@@ -324,7 +287,7 @@ mod tests {
} }
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// Test 8: routine_history // Test 7: routine_history
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
#[tokio::test] #[tokio::test]
@@ -875,4 +838,112 @@ mod tests {
rig.shutdown(); rig.shutdown();
} }
#[tokio::test]
async fn tool_info_clarifies_message_and_channel_setup_roles() {
let trace = LlmTrace::new(
"test-tool-info-channel-message-clarity",
vec![TraceTurn {
user_input: "How do message and channels differ?".to_string(),
steps: vec![
TraceStep {
request_hint: None,
response: TraceResponse::ToolCalls {
tool_calls: vec![TraceToolCall {
id: "call_tool_info_message".to_string(),
name: "tool_info".to_string(),
arguments: serde_json::json!({"name": "message"}),
}],
input_tokens: 100,
output_tokens: 20,
},
expected_tool_results: Vec::new(),
},
TraceStep {
request_hint: None,
response: TraceResponse::ToolCalls {
tool_calls: vec![TraceToolCall {
id: "call_tool_info_tool_search".to_string(),
name: "tool_info".to_string(),
arguments: serde_json::json!({"name": "tool_search"}),
}],
input_tokens: 140,
output_tokens: 20,
},
expected_tool_results: Vec::new(),
},
TraceStep {
request_hint: None,
response: TraceResponse::Text {
content: "I checked both tool descriptions.".to_string(),
input_tokens: 220,
output_tokens: 30,
},
expected_tool_results: Vec::new(),
},
],
expects: Default::default(),
}],
);
let rig = TestRigBuilder::new()
.with_trace(trace.clone())
.with_auto_approve_tools(true)
.build()
.await;
rig.send_message("How do message and channels differ?")
.await;
let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await;
rig.verify_trace_expects(&trace, &responses);
let results = rig.tool_results();
let info_results: Vec<_> = results.iter().filter(|(n, _)| n == "tool_info").collect();
assert_eq!(info_results.len(), 2, "Expected two tool_info results");
let info_json: Vec<serde_json::Value> = info_results
.iter()
.map(|(_, preview)| {
serde_json::from_str(preview)
.expect("tool_info result preview should be valid JSON")
})
.collect();
let message_json = info_json
.iter()
.find(|info| info["name"] == "message")
.expect("tool_info result should contain 'message'");
let message_description = message_json["description"]
.as_str()
.expect("message description should be a string");
assert!(
message_description.contains("Use normal assistant output to reply"),
"message description should distinguish normal replies: {message_description}"
);
assert!(
message_description.contains("proactive notifications"),
"message description should describe proactive sends: {message_description}"
);
let tool_search_json = info_json
.iter()
.find(|info| info["name"] == "tool_search")
.expect("tool_info result should contain 'tool_search'");
let tool_search_description = tool_search_json["description"]
.as_str()
.expect("tool_search description should be a string");
assert!(
tool_search_description.contains("`tool_install`")
&& tool_search_description.contains("`tool_activate`"),
"tool_search description should describe setup/activation via tool_install and \
tool_activate: {tool_search_description}"
);
assert!(
tool_search_description.contains("use the `message` tool for proactive outbound sends"),
"tool_search description should point outbound sends to message: {tool_search_description}"
);
rig.shutdown();
}
} }
@@ -1,70 +0,0 @@
{
"model_name": "test-routine-update-fail-delete-fallback",
"expects": {
"tools_used": ["routine_create", "routine_update", "routine_delete"],
"tool_results_contain": {
"routine_update": "Cannot update schedule or timezone on a non-cron routine.",
"routine_delete": "temp-routine"
},
"min_responses": 1
},
"steps": [
{
"response": {
"type": "tool_calls",
"tool_calls": [
{
"id": "call_rc_fallback",
"name": "routine_create",
"arguments": {
"name": "temp-routine",
"trigger_type": "manual",
"prompt": "Temporary routine for fallback test."
}
}
],
"input_tokens": 120,
"output_tokens": 40
}
},
{
"response": {
"type": "tool_calls",
"tool_calls": [
{
"id": "call_ru_fallback",
"name": "routine_update",
"arguments": {
"name": "temp-routine",
"schedule": "0 */10 * * * *"
}
}
],
"input_tokens": 200,
"output_tokens": 30
}
},
{
"response": {
"type": "tool_calls",
"tool_calls": [
{
"id": "call_rd_fallback",
"name": "routine_delete",
"arguments": {}
}
],
"input_tokens": 300,
"output_tokens": 20
}
},
{
"response": {
"type": "text",
"content": "I recovered from the failed update and cleaned up the original routine.",
"input_tokens": 380,
"output_tokens": 25
}
}
]
}
+43
View File
@@ -237,4 +237,47 @@ mod tests {
rig.shutdown(); rig.shutdown();
} }
#[tokio::test]
async fn telegram_system_prompt_clarifies_reply_vs_proactive_message_tool() {
let trace = simple_trace(1);
let rig = TestRigBuilder::new().with_trace(trace).build().await;
let msg = IncomingMessage::new("telegram", "telegram-user", "Hello there");
rig.send_incoming(msg).await;
let _responses = rig.wait_for_responses(1, TIMEOUT).await;
let requests = rig.captured_llm_requests();
let system_prompt =
extract_system_prompt(&requests).expect("Expected a system prompt in the LLM request");
assert!(
system_prompt.contains("Channels are not separate send-message tools"),
"System prompt should describe channels as setup/integration surfaces.\n\
Actual system prompt:\n{system_prompt}"
);
assert!(
system_prompt
.contains("use normal assistant output to reply in the current conversation"),
"System prompt should route ordinary replies through normal assistant output.\n\
Actual system prompt:\n{system_prompt}"
);
assert!(
system_prompt.contains("respond normally without calling `message`"),
"System prompt should say normal replies do not use the message tool.\n\
Actual system prompt:\n{system_prompt}"
);
assert!(
system_prompt.contains("proactive follow-up in the current conversation"),
"System prompt should reserve omitted channel/target for proactive follow-ups.\n\
Actual system prompt:\n{system_prompt}"
);
assert!(
!system_prompt.contains("omit 'target' to send here"),
"System prompt should not imply the message tool is the default way to reply \
in-thread.\nActual system prompt:\n{system_prompt}"
);
rig.shutdown();
}
} }