From fbce9a5fe357601c2f0dd793fa150ff851407617 Mon Sep 17 00:00:00 2001
From: Illia Polosukhin
Date: Sun, 22 Mar 2026 00:25:54 -0700
Subject: [PATCH 01/31] refactor(llm): move transcription module into src/llm/
(#1559)
* refactor(llm): move transcription module into src/llm/
Transcription is an LLM capability (Whisper, Chat Completions audio).
Move it from a top-level module into src/llm/transcription/ to reflect
this, and update all references across the codebase.
Co-Authored-By: Claude Opus 4.6 (1M context)
* style: fix rustfmt formatting after module move
Co-Authored-By: Claude Opus 4.6 (1M context)
---------
Co-authored-by: Claude Opus 4.6 (1M context)
---
src/agent/agent_loop.rs | 2 +-
src/config/transcription.rs | 15 +++++++++------
src/lib.rs | 1 -
src/llm/mod.rs | 1 +
src/{ => llm}/transcription/chat_completions.rs | 0
src/{ => llm}/transcription/mod.rs | 0
src/{ => llm}/transcription/openai.rs | 0
src/main.rs | 9 +++++----
8 files changed, 16 insertions(+), 12 deletions(-)
rename src/{ => llm}/transcription/chat_completions.rs (100%)
rename src/{ => llm}/transcription/mod.rs (100%)
rename src/{ => llm}/transcription/openai.rs (100%)
diff --git a/src/agent/agent_loop.rs b/src/agent/agent_loop.rs
index 54575ecc..5cbd8166 100644
--- a/src/agent/agent_loop.rs
+++ b/src/agent/agent_loop.rs
@@ -162,7 +162,7 @@ pub struct AgentDeps {
/// HTTP interceptor for trace recording/replay.
pub http_interceptor: Option>,
/// Audio transcription middleware for voice messages.
- pub transcription: Option>,
+ pub transcription: Option>,
/// Document text extraction middleware for PDF, DOCX, PPTX, etc.
pub document_extraction: Option>,
/// Sandbox readiness state for full-job routine dispatch.
diff --git a/src/config/transcription.rs b/src/config/transcription.rs
index fc296c9a..191d2a02 100644
--- a/src/config/transcription.rs
+++ b/src/config/transcription.rs
@@ -89,7 +89,9 @@ impl TranscriptionConfig {
}
/// Create the transcription provider if enabled and configured.
- pub fn create_provider(&self) -> Option> {
+ pub fn create_provider(
+ &self,
+ ) -> Option> {
if !self.enabled {
return None;
}
@@ -103,10 +105,11 @@ impl TranscriptionConfig {
"Audio transcription enabled via Chat Completions API"
);
- let mut provider = crate::transcription::ChatCompletionsTranscriptionProvider::new(
- api_key.clone(),
- )
- .with_model(&self.model);
+ let mut provider =
+ crate::llm::transcription::ChatCompletionsTranscriptionProvider::new(
+ api_key.clone(),
+ )
+ .with_model(&self.model);
if let Some(ref base_url) = self.base_url {
provider = provider.with_base_url(base_url);
@@ -121,7 +124,7 @@ impl TranscriptionConfig {
);
let mut provider =
- crate::transcription::OpenAiWhisperProvider::new(api_key.clone())
+ crate::llm::transcription::OpenAiWhisperProvider::new(api_key.clone())
.with_model(&self.model);
if let Some(ref base_url) = self.base_url {
diff --git a/src/lib.rs b/src/lib.rs
index c87a31b2..9bdce343 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -72,7 +72,6 @@ pub mod skills;
pub mod timezone;
pub mod tools;
pub mod tracing_fmt;
-pub mod transcription;
pub mod tunnel;
pub mod util;
pub mod webhooks;
diff --git a/src/llm/mod.rs b/src/llm/mod.rs
index 141cedf0..64ecd519 100644
--- a/src/llm/mod.rs
+++ b/src/llm/mod.rs
@@ -35,6 +35,7 @@ mod rig_adapter;
pub mod session;
pub mod smart_routing;
mod token_refreshing;
+pub mod transcription;
#[cfg(test)]
mod codex_test_helpers;
diff --git a/src/transcription/chat_completions.rs b/src/llm/transcription/chat_completions.rs
similarity index 100%
rename from src/transcription/chat_completions.rs
rename to src/llm/transcription/chat_completions.rs
diff --git a/src/transcription/mod.rs b/src/llm/transcription/mod.rs
similarity index 100%
rename from src/transcription/mod.rs
rename to src/llm/transcription/mod.rs
diff --git a/src/transcription/openai.rs b/src/llm/transcription/openai.rs
similarity index 100%
rename from src/transcription/openai.rs
rename to src/llm/transcription/openai.rs
diff --git a/src/main.rs b/src/main.rs
index 3fbd0453..23224d0f 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -846,10 +846,11 @@ async fn async_main() -> anyhow::Result<()> {
cost_guard: components.cost_guard,
sse_tx: sse_sender,
http_interceptor,
- transcription: config
- .transcription
- .create_provider()
- .map(|p| Arc::new(ironclaw::transcription::TranscriptionMiddleware::new(p))),
+ transcription: config.transcription.create_provider().map(|p| {
+ Arc::new(ironclaw::llm::transcription::TranscriptionMiddleware::new(
+ p,
+ ))
+ }),
document_extraction: Some(Arc::new(
ironclaw::document_extraction::DocumentExtractionMiddleware::new(),
)),
From 3aa36c8f55c61a9d9fcfabdbbae944ab0a46f130 Mon Sep 17 00:00:00 2001
From: Illia Polosukhin
Date: Sun, 22 Mar 2026 14:36:24 -0700
Subject: [PATCH 02/31] fix(tests): eliminate env mutex poison cascade (#1558)
* fix(tests): eliminate env mutex poison cascade and fix test flakiness
The shared ENV_MUTEX used by ~68 config tests would cascade a single
test panic into failures across every module. Replace all .unwrap() /
.expect() lock acquisitions with a poison-recovering lock_env() helper.
Consolidate rogue module-local ENV_LOCK instances (workspace, orchestrator,
bootstrap) onto the shared global mutex to prevent cross-module races.
Also fixes:
- gateway user_id fallback was hardcoded to "default" instead of owner_id
- test_ironclaw_env_path used LazyLock which is order-dependent
Co-Authored-By: Claude Opus 4.6 (1M context)
* test(helpers): add regression test for lock_env poison recovery
Satisfies the regression-test-check CI gate by adding a test that
intentionally poisons ENV_MUTEX and verifies lock_env() recovers.
Co-Authored-By: Claude Opus 4.6 (1M context)
* fix(ci): detect test changes inside #[cfg(test)] regions
The regression test check relied on git diff -W to expand context to
function boundaries, but git doesn't recognize Rust `mod tests {}` as a
function boundary. Changes to imports, helpers, or lock calls inside
test modules were invisible to the check.
Add a line-level fallback: for each changed .rs file, find where
#[cfg(test)] starts and check if any diff hunk targets a line at or
after that boundary. This catches edits anywhere inside test modules
regardless of git's language awareness.
Co-Authored-By: Claude Opus 4.6 (1M context)
* fix: address PR review feedback
- Clear ENV_MUTEX poison after regression test so it doesn't leave
global state dirty for subsequent tests.
- Fix CI regression-test-check to match #[cfg(test)] only when followed
by `mod` (the test module pattern), avoiding false positives from
standalone #[cfg(test)] items like statics or functions.
Co-Authored-By: Claude Opus 4.6 (1M context)
---------
Co-authored-by: Claude Opus 4.6 (1M context)
---
.github/workflows/regression-test-check.yml | 35 ++++++++++++++
src/bootstrap.rs | 37 ++++++++++-----
src/cli/doctor.rs | 8 ++--
src/cli/oauth_defaults.rs | 24 +++++-----
src/config/builder.rs | 6 +--
src/config/channels.rs | 6 +--
src/config/embeddings.rs | 14 +++---
src/config/helpers.rs | 32 ++++++++++++-
src/config/llm.rs | 52 ++++++++++-----------
src/config/safety.rs | 6 +--
src/config/sandbox.rs | 20 ++------
src/config/search.rs | 14 +++---
src/config/wasm.rs | 6 +--
src/config/workspace.rs | 7 +--
src/db/libsql/workspace.rs | 10 ++--
src/extensions/manager.rs | 24 +++-------
src/llm/oauth_helpers.rs | 6 +--
src/orchestrator/mod.rs | 10 ++--
src/setup/wizard.rs | 18 +++----
19 files changed, 192 insertions(+), 143 deletions(-)
diff --git a/.github/workflows/regression-test-check.yml b/.github/workflows/regression-test-check.yml
index ef1a4d92..75b8eb55 100644
--- a/.github/workflows/regression-test-check.yml
+++ b/.github/workflows/regression-test-check.yml
@@ -121,6 +121,7 @@ jobs:
fi
# Whole-function context: detect edits inside existing test functions.
+ # Uses -W (whole function) which works when git recognises function boundaries.
if git diff "${BASE_REF}...${HEAD_REF}" -W -- '*.rs' | awk '
/^@@/ { if (has_test && has_add) { found=1; exit } has_test=0; has_add=0 }
/^ .*#\[test\]/ || /^ .*#\[tokio::test\]/ || /^ .*#\[cfg\(test\)\]/ || /^ .*mod tests/ { has_test=1 }
@@ -132,6 +133,40 @@ jobs:
exit 0
fi
+ # Line-level check: detect changes inside #[cfg(test)] mod blocks.
+ # git -W relies on function boundary detection which misses Rust mod blocks,
+ # so this fallback checks whether changed line numbers fall within test modules.
+ # We specifically match #[cfg(test)] that is followed by `mod` (same or next
+ # line) to avoid false positives from standalone #[cfg(test)] items like
+ # individual statics or functions.
+ CHANGED_RS=$(echo "$CHANGED_FILES" | grep '\.rs$' || true)
+ if [ -n "$CHANGED_RS" ]; then
+ while IFS= read -r rs_file; do
+ [ -f "$rs_file" ] || continue
+
+ # Find the line where #[cfg(test)] precedes a `mod` declaration.
+ # Handles both `#[cfg(test)] mod tests` (same line) and the two-line form.
+ TEST_MOD_START=$(awk '
+ /^[[:space:]]*#\[cfg\(test\)\].*mod / { print NR; exit }
+ /^[[:space:]]*#\[cfg\(test\)\][[:space:]]*$/ { pending=NR; next }
+ pending && /^[[:space:]]*mod / { print pending; exit }
+ { pending=0 }
+ ' "$rs_file")
+ [ -n "$TEST_MOD_START" ] || continue
+
+ # Get changed line numbers in this file from the diff hunk headers.
+ # Each @@ line looks like: @@ -old,count +new,count @@
+ while IFS= read -r hunk_line; do
+ line_no=$(echo "$hunk_line" | sed -E 's/^@@ -[0-9,]+ \+([0-9]+).*/\1/')
+ [ -n "$line_no" ] || continue
+ if [ "$line_no" -ge "$TEST_MOD_START" ]; then
+ echo "Test changes found: $rs_file has changes at line $line_no inside #[cfg(test)] mod block (starts at line $TEST_MOD_START)."
+ exit 0
+ fi
+ done < <(git diff "${BASE_REF}...${HEAD_REF}" -U0 -- "$rs_file" | grep -E '^@@')
+ done <<< "$CHANGED_RS"
+ fi
+
if grep -qE '^tests/' <<< "$CHANGED_FILES"; then
echo "Test file changes found under tests/."
exit 0
diff --git a/src/bootstrap.rs b/src/bootstrap.rs
index f8a283f3..a5c8ffdb 100644
--- a/src/bootstrap.rs
+++ b/src/bootstrap.rs
@@ -568,14 +568,12 @@ impl Drop for PidLock {
#[cfg(test)]
mod tests {
use super::*;
+ use crate::config::helpers::lock_env;
use std::process::Command;
- use std::sync::Mutex;
use std::thread;
use std::time::{Duration, Instant};
use tempfile::tempdir;
- static ENV_MUTEX: Mutex<()> = Mutex::new(());
-
#[test]
fn test_save_and_load_database_url() {
let dir = tempdir().unwrap();
@@ -669,8 +667,23 @@ INJECTED="pwned"#;
#[test]
fn test_ironclaw_env_path() {
- let path = ironclaw_env_path();
- assert!(path.ends_with(".ironclaw/.env"));
+ // Use compute_ironclaw_base_dir() directly to avoid LazyLock caching,
+ // which can be poisoned by whichever test initializes it first.
+ let _guard = lock_env();
+ let old_val = std::env::var("IRONCLAW_BASE_DIR").ok();
+ // SAFETY: Under lock_env(), no concurrent env access.
+ unsafe { std::env::remove_var("IRONCLAW_BASE_DIR") };
+
+ let path = compute_ironclaw_base_dir().join(".env");
+ assert!(
+ path.ends_with(".ironclaw/.env"),
+ "expected path ending with .ironclaw/.env, got: {}",
+ path.display()
+ );
+
+ if let Some(val) = old_val {
+ unsafe { std::env::set_var("IRONCLAW_BASE_DIR", val) };
+ }
}
#[test]
@@ -836,7 +849,7 @@ INJECTED="pwned"#;
#[test]
fn test_libsql_autodetect_sets_backend_when_db_exists() {
- let _guard = ENV_MUTEX.lock().unwrap();
+ let _guard = lock_env();
let old_val = std::env::var("DATABASE_BACKEND").ok();
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
unsafe { std::env::remove_var("DATABASE_BACKEND") };
@@ -907,7 +920,7 @@ INJECTED="pwned"#;
#[test]
fn test_libsql_autodetect_does_not_override_explicit_backend() {
- let _guard = ENV_MUTEX.lock().unwrap();
+ let _guard = lock_env();
let old_val = std::env::var("DATABASE_BACKEND").ok();
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
unsafe { std::env::set_var("DATABASE_BACKEND", "postgres") };
@@ -1034,7 +1047,7 @@ INJECTED="pwned"#;
fn test_ironclaw_base_dir_default() {
// This test must run first (or in isolation) before the LazyLock is initialized.
// It verifies that when IRONCLAW_BASE_DIR is not set, the default path is used.
- let _guard = ENV_MUTEX.lock().unwrap();
+ let _guard = lock_env();
let old_val = std::env::var("IRONCLAW_BASE_DIR").ok();
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
unsafe { std::env::remove_var("IRONCLAW_BASE_DIR") };
@@ -1054,7 +1067,7 @@ INJECTED="pwned"#;
fn test_ironclaw_base_dir_env_override() {
// This test verifies that when IRONCLAW_BASE_DIR is set,
// the custom path is used. Must run before LazyLock is initialized.
- let _guard = ENV_MUTEX.lock().unwrap();
+ let _guard = lock_env();
let old_val = std::env::var("IRONCLAW_BASE_DIR").ok();
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
unsafe { std::env::set_var("IRONCLAW_BASE_DIR", "/custom/ironclaw/path") };
@@ -1076,7 +1089,7 @@ INJECTED="pwned"#;
fn test_compute_base_dir_env_path_join() {
// Verifies that ironclaw_env_path correctly joins .env to the base dir.
// Uses compute_ironclaw_base_dir directly to avoid LazyLock caching.
- let _guard = ENV_MUTEX.lock().unwrap();
+ let _guard = lock_env();
let old_val = std::env::var("IRONCLAW_BASE_DIR").ok();
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
unsafe { std::env::set_var("IRONCLAW_BASE_DIR", "/my/custom/dir") };
@@ -1098,7 +1111,7 @@ INJECTED="pwned"#;
#[test]
fn test_ironclaw_base_dir_empty_env() {
// Verifies that empty IRONCLAW_BASE_DIR falls back to default.
- let _guard = ENV_MUTEX.lock().unwrap();
+ let _guard = lock_env();
let old_val = std::env::var("IRONCLAW_BASE_DIR").ok();
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
unsafe { std::env::set_var("IRONCLAW_BASE_DIR", "") };
@@ -1120,7 +1133,7 @@ INJECTED="pwned"#;
#[test]
fn test_ironclaw_base_dir_special_chars() {
// Verifies that paths with special characters are handled correctly.
- let _guard = ENV_MUTEX.lock().unwrap();
+ let _guard = lock_env();
let old_val = std::env::var("IRONCLAW_BASE_DIR").ok();
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
unsafe { std::env::set_var("IRONCLAW_BASE_DIR", "/tmp/test_with-special.chars") };
diff --git a/src/cli/doctor.rs b/src/cli/doctor.rs
index 5d13ade6..023ac4e1 100644
--- a/src/cli/doctor.rs
+++ b/src/cli/doctor.rs
@@ -692,7 +692,7 @@ mod tests {
}
}
- let _mutex = crate::config::helpers::ENV_MUTEX.lock().expect("env mutex");
+ let _mutex = crate::config::helpers::lock_env();
let prev = std::env::var("LLM_BACKEND").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
@@ -812,7 +812,7 @@ mod tests {
#[test]
fn check_llm_config_shows_nearai_model_for_nearai_backend() {
- let _guard = crate::config::helpers::ENV_MUTEX.lock().expect("env mutex");
+ let _guard = crate::config::helpers::lock_env();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
std::env::remove_var("LLM_BACKEND");
@@ -839,7 +839,7 @@ mod tests {
#[test]
fn check_embeddings_disabled_by_default_returns_skip() {
- let _guard = crate::config::helpers::ENV_MUTEX.lock().expect("env mutex");
+ let _guard = crate::config::helpers::lock_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::remove_var("EMBEDDING_ENABLED");
@@ -861,7 +861,7 @@ mod tests {
#[test]
fn check_routines_enabled_by_default() {
- let _guard = crate::config::helpers::ENV_MUTEX.lock().expect("env mutex");
+ let _guard = crate::config::helpers::lock_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::remove_var("ROUTINES_ENABLED");
diff --git a/src/cli/oauth_defaults.rs b/src/cli/oauth_defaults.rs
index b4e93704..531d474e 100644
--- a/src/cli/oauth_defaults.rs
+++ b/src/cli/oauth_defaults.rs
@@ -758,7 +758,7 @@ mod tests {
use crate::cli::oauth_defaults::{
builtin_credentials, callback_host, callback_url, is_loopback_host, landing_html,
};
- use crate::config::helpers::ENV_MUTEX;
+ use crate::config::helpers::lock_env;
#[test]
fn test_is_loopback_host() {
@@ -775,7 +775,7 @@ mod tests {
#[test]
fn test_callback_host_default() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original = std::env::var("OAUTH_CALLBACK_HOST").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
@@ -792,7 +792,7 @@ mod tests {
#[test]
fn test_callback_host_env_override() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original_host = std::env::var("OAUTH_CALLBACK_HOST").ok();
let original_url = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
@@ -819,7 +819,7 @@ mod tests {
#[test]
fn test_callback_url_default() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
// Clear both env vars to test default behavior
let original_url = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
let original_host = std::env::var("OAUTH_CALLBACK_HOST").ok();
@@ -843,7 +843,7 @@ mod tests {
#[test]
fn test_callback_url_env_override() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
@@ -1008,7 +1008,7 @@ mod tests {
#[test]
fn test_use_gateway_callback_false_by_default() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
@@ -1024,7 +1024,7 @@ mod tests {
#[test]
fn test_use_gateway_callback_true_for_hosted() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
@@ -1045,7 +1045,7 @@ mod tests {
#[test]
fn test_use_gateway_callback_false_for_localhost() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
@@ -1063,7 +1063,7 @@ mod tests {
#[test]
fn test_use_gateway_callback_false_for_empty() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
@@ -1083,7 +1083,7 @@ mod tests {
fn test_build_platform_state_with_instance() {
use crate::cli::oauth_defaults::{build_platform_state, decode_hosted_oauth_state};
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original = std::env::var("IRONCLAW_INSTANCE_NAME").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
@@ -1107,7 +1107,7 @@ mod tests {
fn test_build_platform_state_without_instance() {
use crate::cli::oauth_defaults::{build_platform_state, decode_hosted_oauth_state};
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original = std::env::var("IRONCLAW_INSTANCE_NAME").ok();
let original_oc = std::env::var("OPENCLAW_INSTANCE_NAME").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
@@ -1134,7 +1134,7 @@ mod tests {
fn test_build_platform_state_with_openclaw_instance() {
use crate::cli::oauth_defaults::{build_platform_state, decode_hosted_oauth_state};
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original_ic = std::env::var("IRONCLAW_INSTANCE_NAME").ok();
let original_oc = std::env::var("OPENCLAW_INSTANCE_NAME").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
diff --git a/src/config/builder.rs b/src/config/builder.rs
index 088db90c..f7bad12c 100644
--- a/src/config/builder.rs
+++ b/src/config/builder.rs
@@ -63,12 +63,12 @@ impl BuilderModeConfig {
#[cfg(test)]
mod tests {
use super::*;
- use crate::config::helpers::ENV_MUTEX;
+ use crate::config::helpers::lock_env;
use crate::settings::Settings;
#[test]
fn resolve_falls_back_to_settings() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let mut settings = Settings::default();
settings.builder.max_iterations = 99;
settings.builder.auto_register = false;
@@ -80,7 +80,7 @@ mod tests {
#[test]
fn env_overrides_settings() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let mut settings = Settings::default();
settings.builder.timeout_secs = 123;
diff --git a/src/config/channels.rs b/src/config/channels.rs
index bc704445..d249dd18 100644
--- a/src/config/channels.rs
+++ b/src/config/channels.rs
@@ -113,7 +113,7 @@ impl ChannelsConfig {
let gateway = if gateway_enabled {
let user_id = optional_env("GATEWAY_USER_ID")?
.or_else(|| cs.gateway_user_id.clone())
- .unwrap_or_else(|| "default".to_string());
+ .unwrap_or_else(|| owner_id.to_string());
Some(GatewayConfig {
host: optional_env("GATEWAY_HOST")?
@@ -236,7 +236,7 @@ fn default_channels_dir() -> PathBuf {
#[cfg(test)]
mod tests {
use crate::config::channels::*;
- use crate::config::helpers::ENV_MUTEX;
+ use crate::config::helpers::lock_env;
use crate::settings::Settings;
#[test]
@@ -395,7 +395,7 @@ mod tests {
#[test]
fn resolve_uses_settings_channel_values_with_owner_scope_user_ids() {
- let _guard = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner());
+ let _guard = lock_env();
let mut settings = Settings::default();
settings.channels.http_enabled = true;
settings.channels.http_host = Some("127.0.0.2".to_string());
diff --git a/src/config/embeddings.rs b/src/config/embeddings.rs
index 68b0ff2c..98183976 100644
--- a/src/config/embeddings.rs
+++ b/src/config/embeddings.rs
@@ -196,7 +196,7 @@ impl EmbeddingsConfig {
#[cfg(test)]
mod tests {
use super::*;
- use crate::config::helpers::ENV_MUTEX;
+ use crate::config::helpers::lock_env;
use crate::settings::{EmbeddingsSettings, Settings};
use crate::testing::credentials::*;
@@ -215,7 +215,7 @@ mod tests {
#[test]
fn embeddings_disabled_not_overridden_by_openai_key() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_embedding_env();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
@@ -245,7 +245,7 @@ mod tests {
#[test]
fn embeddings_enabled_from_settings() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_embedding_env();
let settings = Settings {
@@ -265,7 +265,7 @@ mod tests {
#[test]
fn embeddings_env_override_takes_precedence() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_embedding_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
@@ -294,7 +294,7 @@ mod tests {
#[test]
fn embedding_base_url_parsed_from_env() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_embedding_env();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
@@ -313,7 +313,7 @@ mod tests {
#[test]
fn embedding_base_url_defaults_to_none() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_embedding_env();
let settings = Settings::default();
@@ -326,7 +326,7 @@ mod tests {
#[test]
fn cache_size_zero_rejected() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_embedding_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
diff --git a/src/config/helpers.rs b/src/config/helpers.rs
index dc40fc9f..ff5ee706 100644
--- a/src/config/helpers.rs
+++ b/src/config/helpers.rs
@@ -14,6 +14,16 @@ use crate::config::INJECTED_VARS;
#[cfg(test)]
pub(crate) static ENV_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(());
+/// Acquire the env-var mutex, recovering from poison.
+///
+/// A poisoned mutex means a previous test panicked while holding the lock.
+/// The env state might be slightly stale, but cascading every subsequent
+/// test into a `PoisonError` panic is far worse. Recover and carry on.
+#[cfg(test)]
+pub(crate) fn lock_env() -> std::sync::MutexGuard<'static, ()> {
+ ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner())
+}
+
/// Thread-safe mutable overlay for env vars set at runtime.
///
/// Unlike `INJECTED_VARS` (which is set once at startup from the secrets
@@ -353,7 +363,7 @@ mod tests {
#[test]
fn real_env_var_takes_priority_over_runtime_override() {
- let _guard = ENV_MUTEX.lock().unwrap();
+ let _guard = lock_env();
let key = "IRONCLAW_TEST_ENV_PRIORITY_42";
// Set runtime override
@@ -372,6 +382,26 @@ mod tests {
assert_eq!(env_or_override(key), Some("override_value".to_string()));
}
+ // --- lock_env poison recovery (regression for env mutex cascade) ---
+
+ #[test]
+ fn lock_env_recovers_from_poisoned_mutex() {
+ // Simulate a poisoned mutex: spawn a thread that panics while holding the lock.
+ let _ = std::thread::spawn(|| {
+ let _guard = ENV_MUTEX.lock().unwrap();
+ panic!("intentional poison");
+ })
+ .join();
+
+ // The mutex is now poisoned. lock_env() should recover, not cascade.
+ assert!(ENV_MUTEX.lock().is_err(), "mutex should be poisoned");
+ let _guard = lock_env(); // must not panic
+ drop(_guard);
+
+ // Clean up so this test doesn't leave ENV_MUTEX permanently poisoned.
+ ENV_MUTEX.clear_poison();
+ }
+
// --- validate_base_url tests (regression for #1103) ---
#[test]
diff --git a/src/config/llm.rs b/src/config/llm.rs
index 0976051f..87e4daa5 100644
--- a/src/config/llm.rs
+++ b/src/config/llm.rs
@@ -532,7 +532,7 @@ pub fn default_session_path() -> PathBuf {
#[cfg(test)]
mod tests {
use super::*;
- use crate::config::helpers::ENV_MUTEX;
+ use crate::config::helpers::lock_env;
use crate::settings::Settings;
use crate::testing::credentials::*;
@@ -548,7 +548,7 @@ mod tests {
#[test]
fn openai_compatible_uses_selected_model_when_llm_model_unset() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_compatible_env();
let settings = Settings {
@@ -566,7 +566,7 @@ mod tests {
#[test]
fn openai_compatible_llm_model_env_overrides_selected_model() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_compatible_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
@@ -690,7 +690,7 @@ mod tests {
#[test]
fn ollama_uses_selected_model_when_ollama_model_unset() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_ollama_env();
let settings = Settings {
@@ -707,7 +707,7 @@ mod tests {
#[test]
fn ollama_model_env_overrides_selected_model() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_ollama_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
@@ -733,7 +733,7 @@ mod tests {
#[test]
fn openai_compatible_preserves_dotted_model_name() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_compatible_env();
let settings = Settings {
@@ -754,7 +754,7 @@ mod tests {
#[test]
fn registry_provider_resolves_groq() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::remove_var("LLM_BACKEND");
@@ -779,7 +779,7 @@ mod tests {
#[test]
fn registry_provider_resolves_tinfoil() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::remove_var("LLM_BACKEND");
@@ -807,7 +807,7 @@ mod tests {
#[test]
fn registry_provider_alias_resolves_zai() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::remove_var("LLM_BACKEND");
@@ -832,7 +832,7 @@ mod tests {
#[test]
fn registry_provider_resolves_github_copilot_alias() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::set_var("LLM_BACKEND", "github-copilot");
@@ -880,7 +880,7 @@ mod tests {
#[test]
fn nearai_backend_has_no_registry_provider() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::remove_var("LLM_BACKEND");
@@ -894,7 +894,7 @@ mod tests {
#[test]
fn backend_alias_normalized_to_canonical_id() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_compatible_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
@@ -920,7 +920,7 @@ mod tests {
#[test]
fn unknown_backend_falls_back_to_openai_compatible() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_compatible_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
@@ -944,7 +944,7 @@ mod tests {
#[test]
fn nearai_aliases_all_resolve_to_nearai() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
for alias in &["nearai", "near_ai", "near"] {
// SAFETY: Under ENV_MUTEX.
@@ -971,7 +971,7 @@ mod tests {
#[test]
fn base_url_resolution_priority() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_compatible_env();
// SAFETY: Under ENV_MUTEX.
@@ -1029,7 +1029,7 @@ mod tests {
fn anthropic_oauth_token_sets_placeholder_api_key() {
use secrecy::ExposeSecret;
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_anthropic_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
@@ -1067,7 +1067,7 @@ mod tests {
fn anthropic_api_key_takes_priority_over_oauth() {
use secrecy::ExposeSecret;
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_anthropic_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
@@ -1100,7 +1100,7 @@ mod tests {
#[test]
fn non_anthropic_provider_has_no_oauth_token() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_anthropic_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
@@ -1208,7 +1208,7 @@ mod tests {
#[test]
fn test_request_timeout_defaults_to_120() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::remove_var("LLM_REQUEST_TIMEOUT_SECS");
@@ -1219,7 +1219,7 @@ mod tests {
#[test]
fn test_request_timeout_configurable() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::set_var("LLM_REQUEST_TIMEOUT_SECS", "300");
@@ -1246,7 +1246,7 @@ mod tests {
#[test]
fn openai_codex_resolves_config() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_codex_env();
let settings = Settings {
@@ -1266,7 +1266,7 @@ mod tests {
#[test]
fn openai_codex_model_env_resolution() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_codex_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
@@ -1290,7 +1290,7 @@ mod tests {
#[test]
fn openai_codex_falls_back_to_openai_model() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_codex_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
@@ -1314,7 +1314,7 @@ mod tests {
#[test]
fn openai_codex_falls_back_to_selected_model() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_codex_env();
let settings = Settings {
@@ -1331,7 +1331,7 @@ mod tests {
/// Regression: SSRF validation on OPENAI_CODEX_API_URL (#1103).
#[test]
fn openai_codex_rejects_ssrf_api_url() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_codex_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
@@ -1362,7 +1362,7 @@ mod tests {
/// Regression: SSRF validation on OPENAI_CODEX_AUTH_URL (#1103).
#[test]
fn openai_codex_rejects_ssrf_auth_url() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_openai_codex_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
diff --git a/src/config/safety.rs b/src/config/safety.rs
index ff9e900a..edeceee0 100644
--- a/src/config/safety.rs
+++ b/src/config/safety.rs
@@ -19,12 +19,12 @@ pub(crate) fn resolve_safety_config(
#[cfg(test)]
mod tests {
use super::*;
- use crate::config::helpers::ENV_MUTEX;
+ use crate::config::helpers::lock_env;
use crate::settings::Settings;
#[test]
fn resolve_falls_back_to_settings() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let mut settings = Settings::default();
settings.safety.max_output_length = 42;
settings.safety.injection_check_enabled = false;
@@ -36,7 +36,7 @@ mod tests {
#[test]
fn env_overrides_settings() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let mut settings = Settings::default();
settings.safety.max_output_length = 42;
diff --git a/src/config/sandbox.rs b/src/config/sandbox.rs
index 8c0eb689..01a8c327 100644
--- a/src/config/sandbox.rs
+++ b/src/config/sandbox.rs
@@ -594,9 +594,7 @@ mod tests {
#[test]
fn sandbox_resolve_falls_back_to_settings() {
- let _guard = crate::config::helpers::ENV_MUTEX
- .lock()
- .expect("env mutex poisoned");
+ let _guard = crate::config::helpers::lock_env();
let mut settings = crate::settings::Settings::default();
settings.sandbox.cpu_shares = 99;
settings.sandbox.auto_pull_image = false;
@@ -610,9 +608,7 @@ mod tests {
#[test]
fn sandbox_env_overrides_settings() {
- let _guard = crate::config::helpers::ENV_MUTEX
- .lock()
- .expect("env mutex poisoned");
+ let _guard = crate::config::helpers::lock_env();
let mut settings = crate::settings::Settings::default();
settings.sandbox.timeout_secs = 999;
@@ -628,9 +624,7 @@ mod tests {
#[test]
fn claude_code_resolve_uses_settings_enabled() {
- let _guard = crate::config::helpers::ENV_MUTEX
- .lock()
- .expect("env mutex poisoned");
+ let _guard = crate::config::helpers::lock_env();
let mut settings = crate::settings::Settings::default();
settings.sandbox.claude_code_enabled = true;
@@ -640,9 +634,7 @@ mod tests {
#[test]
fn claude_code_resolve_defaults_disabled() {
- let _guard = crate::config::helpers::ENV_MUTEX
- .lock()
- .expect("env mutex poisoned");
+ let _guard = crate::config::helpers::lock_env();
let settings = crate::settings::Settings::default();
let cfg = ClaudeCodeConfig::resolve(&settings).expect("resolve");
assert!(!cfg.enabled);
@@ -650,9 +642,7 @@ mod tests {
#[test]
fn claude_code_env_overrides_settings() {
- let _guard = crate::config::helpers::ENV_MUTEX
- .lock()
- .expect("env mutex poisoned");
+ let _guard = crate::config::helpers::lock_env();
let mut settings = crate::settings::Settings::default();
settings.sandbox.claude_code_enabled = true;
diff --git a/src/config/search.rs b/src/config/search.rs
index 9555fecc..e6b663cf 100644
--- a/src/config/search.rs
+++ b/src/config/search.rs
@@ -92,7 +92,7 @@ impl WorkspaceSearchConfig {
#[cfg(test)]
mod tests {
use super::*;
- use crate::config::helpers::ENV_MUTEX;
+ use crate::config::helpers::lock_env;
fn clear_search_env() {
// SAFETY: Only called under ENV_MUTEX in tests.
@@ -106,7 +106,7 @@ mod tests {
#[test]
fn defaults_when_no_env() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_search_env();
let config = WorkspaceSearchConfig::resolve().expect("should resolve");
@@ -118,7 +118,7 @@ mod tests {
#[test]
fn env_overrides() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_search_env();
// SAFETY: Under ENV_MUTEX.
@@ -140,7 +140,7 @@ mod tests {
#[test]
fn invalid_strategy_rejected() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_search_env();
// SAFETY: Under ENV_MUTEX.
@@ -156,7 +156,7 @@ mod tests {
#[test]
fn weighted_strategy_defaults() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_search_env();
// SAFETY: Under ENV_MUTEX.
@@ -175,7 +175,7 @@ mod tests {
#[test]
fn weighted_both_zero_rejected() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_search_env();
// SAFETY: Under ENV_MUTEX.
@@ -193,7 +193,7 @@ mod tests {
#[test]
fn rrf_both_zero_allowed() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
clear_search_env();
// SAFETY: Under ENV_MUTEX.
diff --git a/src/config/wasm.rs b/src/config/wasm.rs
index a9bfbd35..4c494a38 100644
--- a/src/config/wasm.rs
+++ b/src/config/wasm.rs
@@ -95,12 +95,12 @@ impl WasmConfig {
#[cfg(test)]
mod tests {
use super::*;
- use crate::config::helpers::ENV_MUTEX;
+ use crate::config::helpers::lock_env;
use crate::settings::Settings;
#[test]
fn resolve_falls_back_to_settings() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let mut settings = Settings::default();
settings.wasm.default_memory_limit = 42;
settings.wasm.cache_compiled = false;
@@ -112,7 +112,7 @@ mod tests {
#[test]
fn env_overrides_settings() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let mut settings = Settings::default();
settings.wasm.default_fuel_limit = 42;
diff --git a/src/config/workspace.rs b/src/config/workspace.rs
index 5f89c655..5daa73eb 100644
--- a/src/config/workspace.rs
+++ b/src/config/workspace.rs
@@ -79,13 +79,10 @@ impl WorkspaceConfig {
#[cfg(test)]
mod tests {
use super::*;
- use std::sync::Mutex;
-
- // Serialize env-var-dependent tests to avoid races.
- static ENV_LOCK: Mutex<()> = Mutex::new(());
+ use crate::config::helpers::lock_env;
fn with_env(key: &str, val: Option<&str>, f: impl FnOnce()) {
- let _guard = ENV_LOCK.lock().unwrap();
+ let _guard = lock_env();
let prev = std::env::var(key).ok();
match val {
Some(v) => unsafe { std::env::set_var(key, v) },
diff --git a/src/db/libsql/workspace.rs b/src/db/libsql/workspace.rs
index d43f1277..5680e435 100644
--- a/src/db/libsql/workspace.rs
+++ b/src/db/libsql/workspace.rs
@@ -1017,7 +1017,7 @@ mod tests {
mod resolve_dimension {
use super::*;
- use crate::config::helpers::ENV_MUTEX;
+ use crate::config::helpers::lock_env;
fn clear_embedding_env() {
// SAFETY: called under ENV_MUTEX
@@ -1030,14 +1030,14 @@ mod tests {
#[test]
fn returns_none_when_disabled() {
- let _guard = ENV_MUTEX.lock().expect("env mutex");
+ let _guard = lock_env();
clear_embedding_env();
assert!(resolve_embedding_dimension().is_none());
}
#[test]
fn returns_explicit_dimension() {
- let _guard = ENV_MUTEX.lock().expect("env mutex");
+ let _guard = lock_env();
clear_embedding_env();
// SAFETY: under ENV_MUTEX
unsafe {
@@ -1053,7 +1053,7 @@ mod tests {
#[test]
fn infers_from_model() {
- let _guard = ENV_MUTEX.lock().expect("env mutex");
+ let _guard = lock_env();
clear_embedding_env();
// SAFETY: under ENV_MUTEX
unsafe {
@@ -1069,7 +1069,7 @@ mod tests {
#[test]
fn defaults_to_1536_for_unknown_model() {
- let _guard = ENV_MUTEX.lock().expect("env mutex");
+ let _guard = lock_env();
clear_embedding_env();
// SAFETY: under ENV_MUTEX
unsafe {
diff --git a/src/extensions/manager.rs b/src/extensions/manager.rs
index 3ecf3657..df5de72d 100644
--- a/src/extensions/manager.rs
+++ b/src/extensions/manager.rs
@@ -7305,9 +7305,7 @@ mod tests {
#[test]
fn should_use_gateway_mode_true_for_tunnel_url() {
- let _guard = crate::config::helpers::ENV_MUTEX
- .lock()
- .expect("env mutex poisoned");
+ let _guard = crate::config::helpers::lock_env();
let original = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
@@ -7329,9 +7327,7 @@ mod tests {
#[test]
fn should_use_gateway_mode_false_without_tunnel() {
- let _guard = crate::config::helpers::ENV_MUTEX
- .lock()
- .expect("env mutex poisoned");
+ let _guard = crate::config::helpers::lock_env();
let original = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
unsafe {
std::env::remove_var("IRONCLAW_OAUTH_CALLBACK_URL");
@@ -7352,9 +7348,7 @@ mod tests {
#[test]
fn should_use_gateway_mode_false_for_loopback_tunnel() {
- let _guard = crate::config::helpers::ENV_MUTEX
- .lock()
- .expect("env mutex poisoned");
+ let _guard = crate::config::helpers::lock_env();
let original = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
unsafe {
std::env::remove_var("IRONCLAW_OAUTH_CALLBACK_URL");
@@ -7382,9 +7376,7 @@ mod tests {
impl EnvGuard {
fn new() -> Self {
- let guard = crate::config::helpers::ENV_MUTEX
- .lock()
- .expect("env mutex poisoned");
+ let guard = crate::config::helpers::lock_env();
let original = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
@@ -7442,9 +7434,7 @@ mod tests {
#[test]
fn gateway_callback_redirect_uri_does_not_duplicate_callback_path_from_env() {
- let _guard = crate::config::helpers::ENV_MUTEX
- .lock()
- .expect("env mutex poisoned");
+ let _guard = crate::config::helpers::lock_env();
let original = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
unsafe {
std::env::set_var(
@@ -7470,9 +7460,7 @@ mod tests {
#[test]
fn gateway_callback_redirect_uri_trims_trailing_slash_from_env_callback() {
- let _guard = crate::config::helpers::ENV_MUTEX
- .lock()
- .expect("env mutex poisoned");
+ let _guard = crate::config::helpers::lock_env();
let original = std::env::var("IRONCLAW_OAUTH_CALLBACK_URL").ok();
unsafe {
std::env::set_var(
diff --git a/src/llm/oauth_helpers.rs b/src/llm/oauth_helpers.rs
index 2881e60e..daaf1b42 100644
--- a/src/llm/oauth_helpers.rs
+++ b/src/llm/oauth_helpers.rs
@@ -361,7 +361,7 @@ pub fn landing_html(provider_name: &str, success: bool) -> String {
#[cfg(test)]
mod tests {
use super::*;
- use crate::config::helpers::ENV_MUTEX;
+ use crate::config::helpers::lock_env;
#[test]
fn loopback_detection() {
@@ -390,7 +390,7 @@ mod tests {
#[allow(clippy::await_holding_lock)]
#[tokio::test]
async fn bind_rejects_wildcard_ipv4() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original = std::env::var("OAUTH_CALLBACK_HOST").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe { std::env::set_var("OAUTH_CALLBACK_HOST", "0.0.0.0") };
@@ -414,7 +414,7 @@ mod tests {
#[allow(clippy::await_holding_lock)]
#[tokio::test]
async fn bind_rejects_wildcard_ipv6() {
- let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
+ let _guard = lock_env();
let original = std::env::var("OAUTH_CALLBACK_HOST").ok();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe { std::env::set_var("OAUTH_CALLBACK_HOST", "::") };
diff --git a/src/orchestrator/mod.rs b/src/orchestrator/mod.rs
index b72f90ee..d6e028a5 100644
--- a/src/orchestrator/mod.rs
+++ b/src/orchestrator/mod.rs
@@ -164,19 +164,15 @@ pub async fn setup_orchestrator(
#[cfg(test)]
mod tests {
- use std::sync::Mutex;
-
use super::*;
-
- /// Serialize access to `ORCHESTRATOR_PORT` env var across test threads.
- static ENV_LOCK: Mutex<()> = Mutex::new(());
+ use crate::config::helpers::lock_env;
#[test]
fn resolve_orchestrator_port_from_env() {
- let _guard = ENV_LOCK.lock().unwrap();
+ let _guard = lock_env();
// Safety: env-var mutation requires unsafe in edition 2024;
- // ENV_LOCK serializes concurrent access from other test threads.
+ // lock_env() serializes concurrent access from other test threads.
// Absent env var → default 50051
unsafe { std::env::remove_var("ORCHESTRATOR_PORT") };
diff --git a/src/setup/wizard.rs b/src/setup/wizard.rs
index b7669070..7ad86610 100644
--- a/src/setup/wizard.rs
+++ b/src/setup/wizard.rs
@@ -3736,7 +3736,7 @@ mod tests {
use tempfile::tempdir;
use super::*;
- use crate::config::helpers::ENV_MUTEX;
+ use crate::config::helpers::lock_env;
#[test]
fn test_wizard_creation() {
@@ -3760,7 +3760,7 @@ mod tests {
#[test]
fn test_wizard_owner_id_uses_resolved_env_scope() {
- let _guard = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner());
+ let _guard = lock_env();
let _owner = EnvGuard::set("IRONCLAW_OWNER_ID", " wizard-owner ");
let wizard = SetupWizard::new();
@@ -3769,7 +3769,7 @@ mod tests {
#[test]
fn test_wizard_owner_id_uses_toml_scope() {
- let _guard = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner());
+ let _guard = lock_env();
let _owner = EnvGuard::clear("IRONCLAW_OWNER_ID");
let dir = tempdir().unwrap(); // safety: test-only tempdir setup
let path = dir.path().join("config.toml");
@@ -3785,7 +3785,7 @@ mod tests {
fn test_try_with_config_and_toml_propagates_invalid_owner_env() {
use std::os::unix::ffi::OsStringExt;
- let _guard = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner());
+ let _guard = lock_env();
let original = std::env::var_os("IRONCLAW_OWNER_ID");
unsafe {
std::env::set_var("IRONCLAW_OWNER_ID", OsString::from_vec(vec![0x66, 0x80]));
@@ -4245,7 +4245,7 @@ mod tests {
fn test_build_nearai_model_fetch_config_picks_up_api_key_env() {
use secrecy::ExposeSecret;
- let _lock = ENV_MUTEX.lock().unwrap();
+ let _lock = lock_env();
let _guard = EnvGuard::set("NEARAI_API_KEY", "test-cloud-api-key-12345");
let _guard2 = EnvGuard::clear("NEARAI_BASE_URL");
@@ -4269,7 +4269,7 @@ mod tests {
/// the config should have `api_key: None` (session token path).
#[test]
fn test_build_nearai_model_fetch_config_none_when_no_api_key() {
- let _lock = ENV_MUTEX.lock().unwrap();
+ let _lock = lock_env();
let _guard = EnvGuard::clear("NEARAI_API_KEY");
let _guard2 = EnvGuard::clear("NEARAI_BASE_URL");
@@ -4288,7 +4288,7 @@ mod tests {
/// Regression test for #799: empty NEARAI_API_KEY should be treated as absent.
#[test]
fn test_build_nearai_model_fetch_config_none_when_empty_api_key() {
- let _lock = ENV_MUTEX.lock().unwrap();
+ let _lock = lock_env();
let _guard = EnvGuard::set("NEARAI_API_KEY", "");
let config = build_nearai_model_fetch_config();
@@ -4306,7 +4306,7 @@ mod tests {
fn test_model_discovery_picks_up_injected_var() {
use secrecy::ExposeSecret;
- let _lock = ENV_MUTEX.lock().unwrap();
+ let _lock = lock_env();
let _guard = EnvGuard::clear("NEARAI_API_KEY");
let _guard2 = EnvGuard::clear("NEARAI_BASE_URL");
@@ -4337,7 +4337,7 @@ mod tests {
/// the NEAR AI authentication menu.
#[test]
fn test_build_nearai_model_fetch_config_picks_up_runtime_env() {
- let _lock = ENV_MUTEX.lock().unwrap();
+ let _lock = lock_env();
// Ensure the real env var is unset so the only source is the overlay.
let _guard = EnvGuard::clear("NEARAI_API_KEY");
From 969b559e2abca655731da98e85ca4b62313f77a7 Mon Sep 17 00:00:00 2001
From: Nige
Date: Sun, 22 Mar 2026 21:41:54 +0000
Subject: [PATCH 03/31] fix(mcp): handle empty 202 notification
acknowledgements (#1539)
* fix(mcp): handle empty 202 notification acknowledgements
* test(mcp): tighten accepted response regression coverage
* Update src/tools/mcp/http_transport.rs
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
---
src/tools/mcp/http_transport.rs | 61 +++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/src/tools/mcp/http_transport.rs b/src/tools/mcp/http_transport.rs
index ec7139c9..59873ce4 100644
--- a/src/tools/mcp/http_transport.rs
+++ b/src/tools/mcp/http_transport.rs
@@ -130,6 +130,16 @@ impl McpTransport for HttpMcpTransport {
)));
}
+ // MCP notifications commonly acknowledge with 202 Accepted and no body.
+ if response.status() == reqwest::StatusCode::ACCEPTED {
+ return Ok(McpResponse {
+ jsonrpc: "2.0".to_string(),
+ id: request.id,
+ result: None,
+ error: None,
+ });
+ }
+
// Determine response format from Content-Type.
let content_type = response
.headers()
@@ -506,4 +516,55 @@ mod tests {
let echoed = response.result.unwrap();
assert_eq!(echoed["authorization"], "Bearer custom-token");
}
+
+ async fn spawn_accepted_server() -> (String, tokio::task::JoinHandle<()>) {
+ use axum::{Router, routing::post};
+ use tokio::net::TcpListener;
+
+ async fn accepted() -> axum::http::StatusCode {
+ axum::http::StatusCode::ACCEPTED
+ }
+
+ let app = Router::new().route("/", post(accepted));
+ let listener = TcpListener::bind("127.0.0.1:0")
+ .await
+ .expect("Failed to bind to an ephemeral port");
+ let addr = listener
+ .local_addr()
+ .expect("Failed to get listener's local address");
+ let url = format!("http://127.0.0.1:{}", addr.port());
+
+ let handle = tokio::spawn(async move {
+ axum::serve(listener, app)
+ .await
+ .expect("Test server failed to run");
+ });
+
+ (url, handle)
+ }
+
+ fn notification_request(method: &str) -> McpRequest {
+ McpRequest {
+ jsonrpc: "2.0".to_string(),
+ id: None,
+ method: method.to_string(),
+ params: None,
+ }
+ }
+
+ #[tokio::test]
+ async fn test_accepted_notification_returns_empty_response() {
+ let (url, _handle) = spawn_accepted_server().await;
+ let transport = HttpMcpTransport::new(&url, "accepted-test");
+ let request = notification_request("notifications/initialized");
+
+ let response = transport
+ .send(&request, &HashMap::new())
+ .await
+ .expect("202 notification response");
+ assert_eq!(response.jsonrpc, "2.0");
+ assert_eq!(response.id, request.id);
+ assert!(response.result.is_none());
+ assert!(response.error.is_none());
+ }
}
From 3e73dbe615683e8a4dec551793df5c85e8e631b9 Mon Sep 17 00:00:00 2001
From: Nige
Date: Mon, 23 Mar 2026 00:48:02 +0000
Subject: [PATCH 04/31] perf(tools): remove unconditional params clone in
shared execution (fix #893) (#926)
* perf(tools): remove unconditional params clone in shared execution
* Update src/tools/execute.rs
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* chore(fmt): apply rustfmt in worker container tool execution
* fix(tools): restore owned param call sites
* fix(tools): pass normalized_params to tool.execute() instead of raw params
The ownership refactor accidentally passed the un-coerced `params` to
`tool.execute()` while validation ran against the coerced
`normalized_params`. This meant tools received un-normalized input
(e.g. stringified JSON arrays instead of actual arrays). Since
`normalized_params` is owned and unused after the execute call, passing
it directly achieves the original zero-clone goal without breaking
parameter coercion.
Co-Authored-By: Claude Opus 4.6 (1M context)
* fix(tools): update empty-tool-name test for owned params signature
Adapts the test_execute_empty_tool_name_returns_not_found test (added
on staging) to pass owned Value instead of &Value, matching the new
execute_tool_with_safety signature.
Co-Authored-By: Claude Opus 4.6 (1M context)
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: ilblackdragon@gmail.com
Co-authored-by: Claude Opus 4.6 (1M context)
---
src/agent/dispatcher.rs | 9 ++++++++-
src/agent/scheduler.rs | 6 +-----
src/tools/execute.rs | 23 ++++++++++-------------
src/worker/container.rs | 11 ++++++++---
4 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/src/agent/dispatcher.rs b/src/agent/dispatcher.rs
index 7fc8e0ca..3f29492d 100644
--- a/src/agent/dispatcher.rs
+++ b/src/agent/dispatcher.rs
@@ -915,7 +915,14 @@ pub(super) async fn execute_chat_tool_standalone(
params: &serde_json::Value,
job_ctx: &crate::context::JobContext,
) -> Result {
- crate::tools::execute::execute_tool_with_safety(tools, safety, tool_name, params, job_ctx).await
+ crate::tools::execute::execute_tool_with_safety(
+ tools,
+ safety,
+ tool_name,
+ params.clone(),
+ job_ctx,
+ )
+ .await
}
/// Parsed auth result fields for emitting StatusUpdate::AuthRequired.
diff --git a/src/agent/scheduler.rs b/src/agent/scheduler.rs
index 2e23b35f..1c4a7fde 100644
--- a/src/agent/scheduler.rs
+++ b/src/agent/scheduler.rs
@@ -549,11 +549,7 @@ impl Scheduler {
// Delegate to shared tool execution pipeline
let output_str = crate::tools::execute::execute_tool_with_safety(
- &tools,
- &safety,
- tool_name,
- &normalized_params,
- &job_ctx,
+ &tools, &safety, tool_name, params, &job_ctx,
)
.await?;
diff --git a/src/tools/execute.rs b/src/tools/execute.rs
index 86da157b..69c72e46 100644
--- a/src/tools/execute.rs
+++ b/src/tools/execute.rs
@@ -19,7 +19,7 @@ pub async fn execute_tool_with_safety(
tools: &ToolRegistry,
safety: &SafetyLayer,
tool_name: &str,
- params: &serde_json::Value,
+ params: serde_json::Value,
job_ctx: &JobContext,
) -> Result {
if tool_name.is_empty() {
@@ -35,7 +35,7 @@ pub async fn execute_tool_with_safety(
name: tool_name.to_string(),
})?;
- let normalized_params = prepare_tool_params(tool.as_ref(), params);
+ let normalized_params = prepare_tool_params(tool.as_ref(), ¶ms);
// Validate tool parameters
let validation = safety.validator().validate_tool_params(&normalized_params);
@@ -63,10 +63,7 @@ pub async fn execute_tool_with_safety(
// Execute with per-tool timeout
let timeout = tool.execution_timeout();
let start = std::time::Instant::now();
- let result = tokio::time::timeout(timeout, async {
- tool.execute(normalized_params.clone(), job_ctx).await
- })
- .await;
+ let result = tokio::time::timeout(timeout, tool.execute(normalized_params, job_ctx)).await;
let elapsed = start.elapsed();
match &result {
@@ -149,7 +146,7 @@ pub async fn execute_tool_simple(
tools: &ToolRegistry,
safety: &SafetyLayer,
tool_name: &str,
- params: &serde_json::Value,
+ params: serde_json::Value,
job_ctx: &JobContext,
) -> Result {
execute_tool_with_safety(tools, safety, tool_name, params, job_ctx)
@@ -308,7 +305,7 @@ mod tests {
®istry,
&safety,
"",
- &serde_json::json!({}),
+ serde_json::json!({}),
&test_job_ctx(),
)
.await;
@@ -331,7 +328,7 @@ mod tests {
let params = serde_json::json!({"message": "hello"});
let result =
- execute_tool_with_safety(®istry, &safety, "echo", ¶ms, &test_job_ctx()).await;
+ execute_tool_with_safety(®istry, &safety, "echo", params, &test_job_ctx()).await;
assert!(result.is_ok(), "Echo tool should succeed");
let output = result.unwrap();
@@ -350,7 +347,7 @@ mod tests {
®istry,
&safety,
"nonexistent",
- &serde_json::json!({}),
+ serde_json::json!({}),
&test_job_ctx(),
)
.await;
@@ -373,7 +370,7 @@ mod tests {
®istry,
&safety,
"fail_tool",
- &serde_json::json!({}),
+ serde_json::json!({}),
&test_job_ctx(),
)
.await;
@@ -397,7 +394,7 @@ mod tests {
®istry,
&safety,
"slow_tool",
- &serde_json::json!({}),
+ serde_json::json!({}),
&test_job_ctx(),
)
.await;
@@ -425,7 +422,7 @@ mod tests {
®istry,
&safety,
"array_echo",
- &serde_json::json!({"values": "[\"1\", \"2\", 3]"}),
+ serde_json::json!({"values": "[\"1\", \"2\", 3]"}),
&test_job_ctx(),
)
.await
diff --git a/src/worker/container.rs b/src/worker/container.rs
index 920cc2ce..e0933975 100644
--- a/src/worker/container.rs
+++ b/src/worker/container.rs
@@ -462,9 +462,14 @@ impl LoopDelegate for ContainerDelegate {
..Default::default()
};
- let result =
- execute_tool_simple(&self.tools, &self.safety, &tc.name, &tc.arguments, &job_ctx)
- .await;
+ let result = execute_tool_simple(
+ &self.tools,
+ &self.safety,
+ &tc.name,
+ tc.arguments.clone(),
+ &job_ctx,
+ )
+ .await;
self.post_event(
"tool_result",
From 7034e910c4741ce0472c9e7b06d1b16ea53ad770 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Niclas=20Overby=20=20=E2=93=83?=
Date: Mon, 23 Mar 2026 02:07:03 +0100
Subject: [PATCH 05/31] fix: generate Mistral-compatible 9-char alphanumeric
tool call IDs (#1242)
* fix: generate Mistral-compatible 9-char alphanumeric tool call IDs
Mistral's API requires tool call IDs to match [a-zA-Z0-9]{9} exactly.
Previously, IDs like 'turn1_0', 'recovered_0', 'call_', and
'generated_tool_call_N' were generated, which Mistral rejects with
HTTP 400.
Add generate_tool_call_id() that produces deterministic 9-char base-36
IDs from two seed values, and use it at all tool call ID generation
sites.
Fixes #1241
* Update src/llm/provider.rs
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix: address review feedback on Mistral tool-call ID generation
- Remove .unwrap() in generate_tool_call_id (provider.rs) per zero-tolerance policy
- Remove .expect() in normalized_tool_call_id (rig_adapter.rs), use direct array indexing
- Replace magic constant 99 with named RECOVERED_TOOL_CALL_SEED in reasoning.rs
- Add tests for normalized_tool_call_id: passthrough, hashing, empty/whitespace, determinism
- Add comment explaining intentional use of turn_idx vs turn.turn_number in session.rs
- Fix duplicate `mod tests` block in provider.rs (pre-existing compile error)
- Update stale test assertions expecting old `generated_tool_call_` prefix format
[skip-regression-check]
* Apply suggestions from code review
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Illia Polosukhin
---
src/agent/dispatcher.rs | 4 +-
src/agent/session.rs | 30 +++++---
src/llm/mod.rs | 2 +-
src/llm/provider.rs | 97 ++++++++++++++++++++++++++
src/llm/reasoning.rs | 24 +++++--
src/llm/rig_adapter.rs | 147 +++++++++++++++++++++++++++++++++++-----
6 files changed, 273 insertions(+), 31 deletions(-)
diff --git a/src/agent/dispatcher.rs b/src/agent/dispatcher.rs
index 3f29492d..03548219 100644
--- a/src/agent/dispatcher.rs
+++ b/src/agent/dispatcher.rs
@@ -1900,7 +1900,7 @@ mod tests {
Ok(ToolCompletionResponse {
content: None,
tool_calls: vec![ToolCall {
- id: format!("call_{}", uuid::Uuid::new_v4()),
+ id: crate::llm::generate_tool_call_id(0, 0),
name: "echo".to_string(),
arguments: serde_json::json!({"message": "looping"}),
}],
@@ -2053,7 +2053,7 @@ mod tests {
Ok(ToolCompletionResponse {
content: None,
tool_calls: vec![ToolCall {
- id: format!("call_{}", uuid::Uuid::new_v4()),
+ id: crate::llm::generate_tool_call_id(0, 0),
name: "nonexistent_tool".to_string(),
arguments: serde_json::json!({}),
}],
diff --git a/src/agent/session.rs b/src/agent/session.rs
index 745b26be..45594922 100644
--- a/src/agent/session.rs
+++ b/src/agent/session.rs
@@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize};
use uuid::Uuid;
use crate::channels::web::util::truncate_preview;
-use crate::llm::{ChatMessage, ToolCall};
+use crate::llm::{ChatMessage, ToolCall, generate_tool_call_id};
/// A session containing one or more threads.
#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -414,7 +414,12 @@ impl Thread {
/// completed actions in subsequent turns.
pub fn messages(&self) -> Vec {
let mut messages = Vec::new();
- for turn in &self.turns {
+ // We use the enumeration index (`turn_idx`) rather than `turn.turn_number`
+ // intentionally: after `truncate_turns()`, the remaining turns are
+ // re-numbered starting from 0, so the enumeration index and turn_number
+ // are equivalent. Using the index avoids coupling to the field and keeps
+ // tool-call ID generation deterministic for the current message window.
+ for (turn_idx, turn) in self.turns.iter().enumerate() {
if turn.image_content_parts.is_empty() {
messages.push(ChatMessage::user(&turn.user_input));
} else {
@@ -425,13 +430,23 @@ impl Thread {
}
if !turn.tool_calls.is_empty() {
- // Build ToolCall objects with synthetic stable IDs
- let tool_calls: Vec = turn
+ // Assign synthetic call IDs for this turn's tool calls, so that
+ // declarations and results can be consistently correlated.
+ let tool_calls_with_ids: Vec<(String, &_)> = turn
.tool_calls
.iter()
.enumerate()
- .map(|(i, tc)| ToolCall {
- id: format!("turn{}_{}", turn.turn_number, i),
+ .map(|(tc_idx, tc)| {
+ // Use provider-compatible tool call IDs derived from turn/tool indices.
+ (generate_tool_call_id(turn_idx, tc_idx), tc)
+ })
+ .collect();
+
+ // Build ToolCall objects using the synthetic call IDs.
+ let tool_calls: Vec = tool_calls_with_ids
+ .iter()
+ .map(|(call_id, tc)| ToolCall {
+ id: call_id.clone(),
name: tc.name.clone(),
arguments: tc.parameters.clone(),
})
@@ -441,8 +456,7 @@ impl Thread {
messages.push(ChatMessage::assistant_with_tool_calls(None, tool_calls));
// Individual tool result messages, truncated to limit context size.
- for (i, tc) in turn.tool_calls.iter().enumerate() {
- let call_id = format!("turn{}_{}", turn.turn_number, i);
+ for (call_id, tc) in tool_calls_with_ids {
let content = if let Some(ref err) = tc.error {
// .error already contains the full error text;
// pass through without wrapping to avoid double-prefix.
diff --git a/src/llm/mod.rs b/src/llm/mod.rs
index 64ecd519..308b3983 100644
--- a/src/llm/mod.rs
+++ b/src/llm/mod.rs
@@ -59,7 +59,7 @@ pub use openai_codex_session::{OpenAiCodexSession, OpenAiCodexSessionManager};
pub use provider::{
ChatMessage, CompletionRequest, CompletionResponse, ContentPart, FinishReason, ImageUrl,
LlmProvider, ModelMetadata, Role, ToolCall, ToolCompletionRequest, ToolCompletionResponse,
- ToolDefinition, ToolResult,
+ ToolDefinition, ToolResult, generate_tool_call_id,
};
pub use reasoning::{
ActionPlan, Reasoning, ReasoningContext, RespondOutput, RespondResult, SILENT_REPLY_TOKEN,
diff --git a/src/llm/provider.rs b/src/llm/provider.rs
index 8a213031..bb45ec68 100644
--- a/src/llm/provider.rs
+++ b/src/llm/provider.rs
@@ -233,6 +233,32 @@ pub struct ToolCall {
pub arguments: serde_json::Value,
}
+/// Generate a tool-call ID that satisfies all providers.
+///
+/// Mistral requires exactly 9 alphanumeric characters (`[a-zA-Z0-9]{9}`).
+/// Other providers accept any non-empty string. By default we produce a
+/// 9-char base-62 string derived from two seed values so the ID is both
+/// deterministic (for replayed history) and provider-compatible.
+pub fn generate_tool_call_id(seed_a: usize, seed_b: usize) -> String {
+ // Mix the two seeds into a single u64 using a simple hash-like combine.
+ let combined = (seed_a as u64)
+ .wrapping_mul(6364136223846793005)
+ .wrapping_add(seed_b as u64);
+ // Format as 9-char zero-padded base-62 (0-9, a-z, A-Z).
+ let mut buf = [b'0'; 9];
+ let mut val = combined;
+ for b in buf.iter_mut().rev() {
+ let digit = (val % 62) as u8;
+ *b = match digit {
+ 0..=9 => b'0' + digit,
+ 10..=35 => b'a' + (digit - 10),
+ _ => b'A' + (digit - 36),
+ };
+ val /= 62;
+ }
+ buf.iter().map(|&b| b as char).collect::()
+}
+
/// Result of a tool execution to send back to the LLM.
#[derive(Debug, Clone)]
pub struct ToolResult {
@@ -533,6 +559,77 @@ pub fn strip_unsupported_tool_params(
#[cfg(test)]
mod tests {
use super::*;
+ use std::collections::HashSet;
+
+ #[test]
+ fn generate_tool_call_id_has_valid_format() {
+ let samples = [
+ (0usize, 0usize),
+ (1usize, 2usize),
+ (42usize, 999usize),
+ (usize::MAX, usize::MAX),
+ ];
+
+ for (a, b) in samples {
+ let id = generate_tool_call_id(a, b);
+ assert_eq!(
+ id.len(),
+ 9,
+ "tool-call ID must be exactly 9 characters for seeds ({a}, {b})"
+ );
+ assert!(
+ id.chars().all(|c| c.is_ascii_alphanumeric()),
+ "tool-call ID must be ASCII alphanumeric for seeds ({a}, {b}), got: {id}"
+ );
+ }
+ }
+
+ #[test]
+ fn generate_tool_call_id_is_deterministic_for_same_seeds() {
+ let pairs = [
+ (0usize, 0usize),
+ (1usize, 2usize),
+ (123usize, 456usize),
+ (usize::MAX, 0usize),
+ ];
+
+ for (a, b) in pairs {
+ let id1 = generate_tool_call_id(a, b);
+ let id2 = generate_tool_call_id(a, b);
+ let id3 = generate_tool_call_id(a, b);
+ assert_eq!(
+ id1, id2,
+ "tool-call ID must be deterministic for seeds ({a}, {b})"
+ );
+ assert_eq!(
+ id2, id3,
+ "tool-call ID must be deterministic across multiple calls for seeds ({a}, {b})"
+ );
+ }
+ }
+
+ #[test]
+ fn generate_tool_call_id_differs_for_different_seeds_in_small_sample() {
+ let seed_pairs = [
+ (0usize, 1usize),
+ (1usize, 0usize),
+ (1usize, 2usize),
+ (2usize, 3usize),
+ (10usize, 20usize),
+ (100usize, 200usize),
+ ];
+
+ let mut ids = HashSet::new();
+ for (a, b) in seed_pairs {
+ let id = generate_tool_call_id(a, b);
+ let inserted = ids.insert(id.clone());
+ assert!(
+ inserted,
+ "expected distinct tool-call IDs for different seeds, \
+ but duplicate ID '{id}' found for seeds ({a}, {b})"
+ );
+ }
+ }
#[test]
fn test_sanitize_preserves_valid_pairs() {
diff --git a/src/llm/reasoning.rs b/src/llm/reasoning.rs
index b00948ae..cbec297b 100644
--- a/src/llm/reasoning.rs
+++ b/src/llm/reasoning.rs
@@ -23,6 +23,13 @@ You said you would perform an action, but you did not include any tool calls.\n\
Do NOT describe what you intend to do — actually call the tool now.\n\
Use the tool_calls mechanism to invoke the appropriate tool.";
+/// Seed value used as the second argument to `generate_tool_call_id` when
+/// recovering tool calls from malformed LLM text responses. This must differ
+/// from the `0` seed used in `rig_adapter::normalized_tool_call_id` to avoid
+/// ID collisions between provider-generated and text-recovered tool calls at
+/// the same positional index.
+const RECOVERED_TOOL_CALL_SEED: usize = 99;
+
/// Detect when an LLM response expresses intent to call a tool without
/// actually issuing tool calls. Returns `true` if the text contains phrases
/// like "Let me search …" or "I'll fetch …" outside of fenced/indented code blocks.
@@ -1337,7 +1344,10 @@ fn recover_tool_calls_from_content(
.cloned()
.unwrap_or(serde_json::Value::Object(Default::default()));
calls.push(ToolCall {
- id: format!("recovered_{}", calls.len()),
+ id: super::provider::generate_tool_call_id(
+ calls.len(),
+ RECOVERED_TOOL_CALL_SEED,
+ ),
name: name.to_string(),
arguments,
});
@@ -1348,7 +1358,10 @@ fn recover_tool_calls_from_content(
let name = inner.trim();
if tool_names.contains(name) {
calls.push(ToolCall {
- id: format!("recovered_{}", calls.len()),
+ id: super::provider::generate_tool_call_id(
+ calls.len(),
+ RECOVERED_TOOL_CALL_SEED,
+ ),
name: name.to_string(),
arguments: serde_json::Value::Object(Default::default()),
});
@@ -1382,7 +1395,10 @@ fn recover_tool_calls_from_content(
let arguments = serde_json::from_str::(args_str)
.unwrap_or(serde_json::Value::Object(Default::default()));
calls.push(ToolCall {
- id: format!("recovered_{}", calls.len()),
+ id: super::provider::generate_tool_call_id(
+ calls.len(),
+ RECOVERED_TOOL_CALL_SEED,
+ ),
name: name.to_string(),
arguments,
});
@@ -1393,7 +1409,7 @@ fn recover_tool_calls_from_content(
// No arguments or malformed — call with empty args
calls.push(ToolCall {
- id: format!("recovered_{}", calls.len()),
+ id: super::provider::generate_tool_call_id(calls.len(), RECOVERED_TOOL_CALL_SEED),
name: name.to_string(),
arguments: serde_json::Value::Object(Default::default()),
});
diff --git a/src/llm/rig_adapter.rs b/src/llm/rig_adapter.rs
index 1741e860..a9030929 100644
--- a/src/llm/rig_adapter.rs
+++ b/src/llm/rig_adapter.rs
@@ -20,6 +20,7 @@ use rust_decimal_macros::dec;
use serde::Serialize;
use serde::de::DeserializeOwned;
use serde_json::Value as JsonValue;
+use sha2::{Digest, Sha256};
use std::collections::HashSet;
@@ -400,11 +401,48 @@ fn convert_messages(messages: &[ChatMessage]) -> (Option, Vec, seed: usize) -> String {
- match raw.map(str::trim).filter(|id| !id.is_empty()) {
- Some(id) => id.to_string(),
- None => format!("generated_tool_call_{seed}"),
+ // Trim and treat empty as None.
+ let trimmed = raw.and_then(|s| {
+ let t = s.trim();
+ if t.is_empty() { None } else { Some(t) }
+ });
+
+ if let Some(id) = trimmed {
+ // If the ID already satisfies `[a-zA-Z0-9]{9}`, pass it through unchanged.
+ if id.len() == 9 && id.chars().all(|c| c.is_ascii_alphanumeric()) {
+ return id.to_string();
+ }
+
+ // Otherwise, deterministically hash the raw ID and feed the hash-derived
+ // seed into the provider-level generator so that the encoding and any
+ // provider-specific constraints remain centralized in one place.
+ let digest = Sha256::digest(id.as_bytes());
+ // Derive a 64-bit value from the first 8 bytes of the digest, then
+ // split it into two usize seeds so we preserve all 64 bits of entropy
+ // even on 32-bit targets.
+ let hash64 = {
+ // SHA-256 always produces 32 bytes, so indexing the first 8 is safe.
+ let bytes: [u8; 8] = [
+ digest[0], digest[1], digest[2], digest[3], digest[4], digest[5], digest[6],
+ digest[7],
+ ];
+ u64::from_be_bytes(bytes)
+ };
+ let hi_seed: usize = (hash64 >> 32) as usize;
+ let lo_seed: usize = (hash64 & 0xFFFF_FFFF) as usize;
+ return super::provider::generate_tool_call_id(hi_seed, lo_seed);
}
+
+ // Fallback for missing/empty raw IDs: use the provider-level generator,
+ // which already produces compliant IDs.
+ super::provider::generate_tool_call_id(seed, 0)
}
/// Convert IronClaw tool definitions to rig-core format.
@@ -813,8 +851,9 @@ mod tests {
#[test]
fn test_convert_messages_tool_result() {
+ // Use a conforming 9-char alphanumeric ID so it passes through unchanged.
let messages = vec![ChatMessage::tool_result(
- "call_123",
+ "abcDE1234",
"search",
"result text",
)];
@@ -825,8 +864,8 @@ mod tests {
match &history[0] {
RigMessage::User { content } => match content.first() {
UserContent::ToolResult(r) => {
- assert_eq!(r.id, "call_123");
- assert_eq!(r.call_id.as_deref(), Some("call_123"));
+ assert_eq!(r.id, "abcDE1234");
+ assert_eq!(r.call_id.as_deref(), Some("abcDE1234"));
}
other => panic!("Expected tool result content, got: {:?}", other),
},
@@ -836,8 +875,9 @@ mod tests {
#[test]
fn test_convert_messages_assistant_with_tool_calls() {
+ // Use a conforming 9-char alphanumeric ID so it passes through unchanged.
let tc = IronToolCall {
- id: "call_1".to_string(),
+ id: "Xt7mK9pQ2".to_string(),
name: "search".to_string(),
arguments: serde_json::json!({"query": "test"}),
};
@@ -851,7 +891,7 @@ mod tests {
assert!(content.iter().count() >= 2);
for item in content.iter() {
if let AssistantContent::ToolCall(tc) = item {
- assert_eq!(tc.call_id.as_deref(), Some("call_1"));
+ assert_eq!(tc.call_id.as_deref(), Some("Xt7mK9pQ2"));
}
}
}
@@ -873,7 +913,14 @@ mod tests {
match &history[0] {
RigMessage::User { content } => match content.first() {
UserContent::ToolResult(r) => {
- assert!(r.id.starts_with("generated_tool_call_"));
+ // Missing ID → normalized_tool_call_id generates a 9-char alphanumeric ID.
+ assert_eq!(
+ r.id.len(),
+ 9,
+ "fallback ID should be 9 chars, got: {}",
+ r.id
+ );
+ assert!(r.id.chars().all(|c| c.is_ascii_alphanumeric()));
assert_eq!(r.call_id.as_deref(), Some(r.id.as_str()));
}
other => panic!("Expected tool result content, got: {:?}", other),
@@ -961,12 +1008,14 @@ mod tests {
_ => None,
});
let tc = tool_call.expect("should have a tool call");
- assert!(!tc.id.is_empty(), "tool call id must not be empty");
- assert!(
- tc.id.starts_with("generated_tool_call_"),
- "empty id should be replaced with generated id, got: {}",
+ // Empty ID → normalized_tool_call_id generates a 9-char alphanumeric ID.
+ assert_eq!(
+ tc.id.len(),
+ 9,
+ "generated id should be 9 chars, got: {}",
tc.id
);
+ assert!(tc.id.chars().all(|c| c.is_ascii_alphanumeric()));
assert_eq!(tc.call_id.as_deref(), Some(tc.id.as_str()));
}
other => panic!("Expected Assistant message, got: {:?}", other),
@@ -990,11 +1039,14 @@ mod tests {
_ => None,
});
let tc = tool_call.expect("should have a tool call");
- assert!(
- tc.id.starts_with("generated_tool_call_"),
- "whitespace-only id should be replaced, got: {:?}",
+ // Whitespace-only ID → normalized_tool_call_id generates a 9-char alphanumeric ID.
+ assert_eq!(
+ tc.id.len(),
+ 9,
+ "generated id should be 9 chars, got: {}",
tc.id
);
+ assert!(tc.id.chars().all(|c| c.is_ascii_alphanumeric()));
}
other => panic!("Expected Assistant message, got: {:?}", other),
}
@@ -1381,4 +1433,67 @@ mod tests {
// Should be 2 separate User messages (text user + tool result user)
assert_eq!(history.len(), 2);
}
+
+ // -- normalized_tool_call_id tests --
+
+ #[test]
+ fn test_normalized_tool_call_id_conforming_passthrough() {
+ // A 9-char alphanumeric ID should pass through unchanged.
+ let id = normalized_tool_call_id(Some("abcDE1234"), 42);
+ assert_eq!(id, "abcDE1234");
+ }
+
+ #[test]
+ fn test_normalized_tool_call_id_non_conforming_hashed() {
+ // An ID that doesn't match [a-zA-Z0-9]{9} should be hashed into one.
+ let id = normalized_tool_call_id(Some("call_abc_long_id"), 0);
+ assert_eq!(id.len(), 9);
+ assert!(id.chars().all(|c| c.is_ascii_alphanumeric()));
+ // Should NOT be the raw input.
+ assert_ne!(id, "call_abc_l");
+ }
+
+ #[test]
+ fn test_normalized_tool_call_id_empty_input() {
+ let id = normalized_tool_call_id(Some(""), 5);
+ assert_eq!(id.len(), 9);
+ assert!(id.chars().all(|c| c.is_ascii_alphanumeric()));
+ }
+
+ #[test]
+ fn test_normalized_tool_call_id_whitespace_input() {
+ let id = normalized_tool_call_id(Some(" "), 5);
+ assert_eq!(id.len(), 9);
+ assert!(id.chars().all(|c| c.is_ascii_alphanumeric()));
+ // Empty and whitespace-only with the same seed should produce identical results.
+ let id_empty = normalized_tool_call_id(Some(""), 5);
+ assert_eq!(id, id_empty);
+ }
+
+ #[test]
+ fn test_normalized_tool_call_id_none_input() {
+ let id = normalized_tool_call_id(None, 7);
+ assert_eq!(id.len(), 9);
+ assert!(id.chars().all(|c| c.is_ascii_alphanumeric()));
+ // None and empty string with same seed should produce identical results.
+ let id_empty = normalized_tool_call_id(Some(""), 7);
+ assert_eq!(id, id_empty);
+ }
+
+ #[test]
+ fn test_normalized_tool_call_id_deterministic() {
+ let id1 = normalized_tool_call_id(Some("call_xyz_123"), 0);
+ let id2 = normalized_tool_call_id(Some("call_xyz_123"), 0);
+ assert_eq!(id1, id2, "same input must produce same output");
+ }
+
+ #[test]
+ fn test_normalized_tool_call_id_different_inputs_differ() {
+ let id_a = normalized_tool_call_id(Some("call_aaa"), 0);
+ let id_b = normalized_tool_call_id(Some("call_bbb"), 0);
+ assert_ne!(
+ id_a, id_b,
+ "different raw IDs should produce different hashed IDs"
+ );
+ }
}
From abba083147775f7d4b03a51a376cb1b7617cf7d1 Mon Sep 17 00:00:00 2001
From: Nige
Date: Mon, 23 Mar 2026 01:27:10 +0000
Subject: [PATCH 06/31] docs(feishu): clarify webhook-only event subscription
support (#1567)
* docs(feishu): clarify webhook-only event subscription support
* Update channels-src/feishu/feishu.capabilities.json
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
---
channels-src/feishu/feishu.capabilities.json | 8 ++++----
channels-src/feishu/src/lib.rs | 4 +++-
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/channels-src/feishu/feishu.capabilities.json b/channels-src/feishu/feishu.capabilities.json
index 82b1be4e..877a293a 100644
--- a/channels-src/feishu/feishu.capabilities.json
+++ b/channels-src/feishu/feishu.capabilities.json
@@ -3,11 +3,11 @@
"wit_version": "0.3.0",
"type": "channel",
"name": "feishu",
- "description": "Feishu/Lark Bot channel for receiving and responding to Feishu messages",
+ "description": "Feishu/Lark Bot channel for receiving and responding to Feishu messages via Event Subscription webhooks",
"auth": {
"secret_name": "feishu_app_id",
"display_name": "Feishu / Lark",
- "instructions": "Create a bot at https://open.feishu.cn/app (Feishu) or https://open.larksuite.com/app (Lark). You need the App ID and App Secret.",
+ "instructions": "Create a bot at https://open.feishu.cn/app (Feishu) or https://open.larksuite.com/app (Lark). You need the App ID and App Secret. Note: IronClaw supports Event Subscription webhook delivery, but not Feishu's long-connection websocket mode.",
"setup_url": "https://open.feishu.cn/app",
"token_hint": "App ID looks like cli_XXXX, App Secret is a long alphanumeric string",
"env_var": "FEISHU_APP_ID"
@@ -16,7 +16,7 @@
"required_secrets": [
{
"name": "feishu_app_id",
- "prompt": "Enter your Feishu/Lark App ID (from https://open.feishu.cn/app)",
+ "prompt": "Enter your Feishu/Lark App ID (from https://open.feishu.cn/app). Use webhook-based Event Subscription, not long-connection websocket mode.",
"optional": false
},
{
@@ -26,7 +26,7 @@
},
{
"name": "feishu_verification_token",
- "prompt": "Enter your Feishu/Lark Verification Token (from Event Subscription settings)",
+ "prompt": "Enter your Feishu/Lark Verification Token (from Event Subscription webhook settings)",
"optional": true
}
],
diff --git a/channels-src/feishu/src/lib.rs b/channels-src/feishu/src/lib.rs
index 3094eaa0..62440d2c 100644
--- a/channels-src/feishu/src/lib.rs
+++ b/channels-src/feishu/src/lib.rs
@@ -5,7 +5,9 @@
//!
//! This WASM component implements the channel interface for handling Feishu
//! webhooks (Event Subscription v2.0) and sending messages back via the
-//! Feishu/Lark Bot API.
+//! Feishu/Lark Bot API. IronClaw currently does not connect to Feishu's
+//! long-connection websocket subscription mode; use Event Subscription
+//! webhooks for this channel.
//!
//! # Features
//!
From 4d7501a9684469998f2b518f6bd3da8bc95b266a Mon Sep 17 00:00:00 2001
From: Henry Park
Date: Sun, 22 Mar 2026 20:33:52 -0700
Subject: [PATCH 07/31] Fix owner-scoped message routing fallbacks (#1574)
* Fix owner-scoped message routing fallbacks
* Address PR feedback on routing regressions
* Address review notes on routing fallbacks
---
src/testing/mod.rs | 71 ++++++++++++++-
src/tools/builtin/message.rs | 167 ++++++++++++++++-------------------
src/worker/job.rs | 65 ++++++++++++++
3 files changed, 211 insertions(+), 92 deletions(-)
diff --git a/src/testing/mod.rs b/src/testing/mod.rs
index 953cbfcd..a633e91c 100644
--- a/src/testing/mod.rs
+++ b/src/testing/mod.rs
@@ -28,7 +28,7 @@ use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
use async_trait::async_trait;
use rust_decimal::Decimal;
-use tokio::sync::mpsc;
+use tokio::sync::{Mutex as AsyncMutex, mpsc};
use crate::agent::AgentDeps;
use crate::channels::{
@@ -361,6 +361,75 @@ impl Channel for StubChannel {
}
}
+/// Captured broadcast deliveries keyed by the target user or chat identifier.
+pub type BroadcastCapture = Arc>>;
+
+/// A lightweight channel double that only records `broadcast()` traffic.
+///
+/// This is useful for unit tests that need to assert message routing without
+/// spinning up a full interactive channel harness.
+pub struct RecordingBroadcastChannel {
+ name: &'static str,
+ captures: BroadcastCapture,
+}
+
+impl RecordingBroadcastChannel {
+ pub fn new(name: &'static str) -> (Self, BroadcastCapture) {
+ let captures = Arc::new(AsyncMutex::new(Vec::new()));
+ (
+ Self {
+ name,
+ captures: Arc::clone(&captures),
+ },
+ captures,
+ )
+ }
+}
+
+#[async_trait]
+impl Channel for RecordingBroadcastChannel {
+ fn name(&self) -> &str {
+ self.name
+ }
+
+ async fn start(&self) -> Result {
+ let (_tx, rx) = mpsc::channel::(1);
+ Ok(Box::pin(tokio_stream::wrappers::ReceiverStream::new(rx)))
+ }
+
+ async fn respond(
+ &self,
+ _msg: &IncomingMessage,
+ _response: OutgoingResponse,
+ ) -> Result<(), ChannelError> {
+ Ok(())
+ }
+
+ async fn send_status(
+ &self,
+ _status: StatusUpdate,
+ _metadata: &serde_json::Value,
+ ) -> Result<(), ChannelError> {
+ Ok(())
+ }
+
+ async fn broadcast(
+ &self,
+ user_id: &str,
+ response: OutgoingResponse,
+ ) -> Result<(), ChannelError> {
+ self.captures
+ .lock()
+ .await
+ .push((user_id.to_string(), response));
+ Ok(())
+ }
+
+ async fn health_check(&self) -> Result<(), ChannelError> {
+ Ok(())
+ }
+}
+
/// Assembled test components.
pub struct TestHarness {
/// The agent dependencies, ready for use.
diff --git a/src/tools/builtin/message.rs b/src/tools/builtin/message.rs
index 83041b80..08029d6f 100644
--- a/src/tools/builtin/message.rs
+++ b/src/tools/builtin/message.rs
@@ -80,6 +80,12 @@ fn metadata_notify_user(metadata: &serde_json::Value) -> Option {
metadata_string(metadata, "notify_user").filter(|value| value != "default")
}
+// Autonomous runs include `owner_id` when the job is executing on behalf of a
+// durable owner scope instead of an interactive channel actor.
+fn metadata_owner_id(metadata: &serde_json::Value) -> Option {
+ metadata_string(metadata, "owner_id")
+}
+
fn channel_matches_source(resolved_channel: Option<&str>, source_channel: Option<&str>) -> bool {
match (resolved_channel, source_channel) {
(None, _) => true,
@@ -91,11 +97,13 @@ fn channel_matches_source(resolved_channel: Option<&str>, source_channel: Option
async fn resolve_channel_fallback_target(
extension_manager: Option<&Arc>,
channel: Option<&str>,
+ owner_scope_target: Option<&str>,
ctx_user_id: &str,
) -> Option {
- let channel_name = channel?;
-
- if let Some(extension_manager) = extension_manager
+ // Prefer an explicit channel binding when the extension manager knows the
+ // durable delivery target (for example, a bound Telegram chat ID).
+ if let Some(channel_name) = channel
+ && let Some(extension_manager) = extension_manager
&& let Some(target) = extension_manager
.notification_target_for_channel(channel_name)
.await
@@ -103,13 +111,19 @@ async fn resolve_channel_fallback_target(
return Some(target);
}
- Some(ctx_user_id.to_string())
+ // `owner_id` is only present for autonomous owner-scoped executions.
+ // Interactive chat turns intentionally fall back to `ctx.user_id`, which is
+ // already the active conversation target for the current channel.
+ owner_scope_target
+ .map(ToOwned::to_owned)
+ .or_else(|| Some(ctx_user_id.to_string()))
}
struct MessageTargetResolution<'a> {
extension_manager: Option<&'a Arc>,
explicit_target: Option,
metadata_target: Option,
+ owner_scope_target: Option,
default_target: Option,
channel: Option<&'a str>,
metadata_channel: Option<&'a str>,
@@ -133,6 +147,7 @@ async fn resolve_message_target(inputs: MessageTargetResolution<'_>) -> Option) -> Option>>;
-
- struct RecordingChannel {
- name: &'static str,
- captures: BroadcastCapture,
- }
-
- impl RecordingChannel {
- fn new(name: &'static str) -> (Self, BroadcastCapture) {
- let captures = Arc::new(Mutex::new(Vec::new()));
- (
- Self {
- name,
- captures: Arc::clone(&captures),
- },
- captures,
- )
- }
- }
-
- #[async_trait]
- impl Channel for RecordingChannel {
- fn name(&self) -> &str {
- self.name
- }
-
- async fn start(&self) -> Result {
- let (_tx, rx) = mpsc::channel::(1);
- Ok(Box::pin(tokio_stream::wrappers::ReceiverStream::new(rx)))
- }
-
- async fn respond(
- &self,
- _msg: &IncomingMessage,
- _response: OutgoingResponse,
- ) -> Result<(), ChannelError> {
- Ok(())
- }
-
- async fn send_status(
- &self,
- _status: StatusUpdate,
- _metadata: &serde_json::Value,
- ) -> Result<(), ChannelError> {
- Ok(())
- }
-
- async fn broadcast(
- &self,
- user_id: &str,
- response: OutgoingResponse,
- ) -> Result<(), ChannelError> {
- self.captures
- .lock()
- .await
- .push((user_id.to_string(), response));
- Ok(())
- }
-
- async fn health_check(&self) -> Result<(), ChannelError> {
- Ok(())
- }
- }
+ use crate::testing::{BroadcastCapture, RecordingBroadcastChannel};
async fn message_tool_with_recording_channels()
-> (MessageTool, BroadcastCapture, BroadcastCapture) {
let channel_manager = ChannelManager::new();
- let (gateway, gateway_captures) = RecordingChannel::new("gateway");
- let (telegram, telegram_captures) = RecordingChannel::new("telegram");
+ let (gateway, gateway_captures) = RecordingBroadcastChannel::new("gateway");
+ let (telegram, telegram_captures) = RecordingBroadcastChannel::new("telegram");
channel_manager.add(Box::new(gateway)).await;
channel_manager.add(Box::new(telegram)).await;
@@ -870,28 +820,63 @@ mod tests {
}
#[tokio::test]
- async fn message_tool_falls_back_to_ctx_user_when_channel_known() {
- // Regression for owner-scoped notifications: a channel can be known
- // even when the concrete delivery target is omitted, so the message
- // tool should pass ctx.user_id through to the channel layer.
- let tool = MessageTool::new(Arc::new(ChannelManager::new()));
+ async fn message_tool_falls_back_to_owner_scope_when_channel_known() {
+ let (tool, gateway_captures, telegram_captures) =
+ message_tool_with_recording_channels().await;
let mut ctx =
- crate::context::JobContext::with_user("owner-scope", "routine-job", "price alert");
+ crate::context::JobContext::with_user("telegram", "routine-job", "price alert");
+ ctx.metadata = serde_json::json!({
+ "notify_channel": "telegram",
+ "owner_id": "owner-scope",
+ });
+
+ let result = tool
+ .execute(serde_json::json!({"content": "NEAR price is $5"}), &ctx)
+ .await
+ .expect("message tool should use owner scope before ctx.user_id");
+
+ assert_eq!(
+ result.result.as_str(),
+ Some("Sent message to telegram:owner-scope")
+ );
+ assert!(gateway_captures.lock().await.is_empty());
+ let telegram = telegram_captures.lock().await.clone();
+ assert_eq!(telegram.len(), 1);
+ assert_eq!(telegram[0].0, "owner-scope");
+ assert_eq!(telegram[0].1.content, "NEAR price is $5");
+ }
+
+ #[tokio::test]
+ async fn message_tool_falls_back_to_ctx_user_when_owner_scope_absent() {
+ let (tool, gateway_captures, telegram_captures) =
+ message_tool_with_recording_channels().await;
+
+ let mut ctx = crate::context::JobContext::with_user(
+ "interactive-chat-user",
+ "routine-job",
+ "price alert",
+ );
ctx.metadata = serde_json::json!({
"notify_channel": "telegram",
});
let result = tool
.execute(serde_json::json!({"content": "NEAR price is $5"}), &ctx)
- .await;
+ .await
+ .expect(
+ "message tool should fall back to ctx.user_id when owner scope metadata is absent",
+ );
- assert!(result.is_err()); // safety: test-only assertion
- let err = result.unwrap_err().to_string();
- let mentions_missing_target = err.contains("No target specified");
- assert!(!mentions_missing_target); // safety: test-only assertion
- let mentions_missing_channel = err.contains("No channel specified");
- assert!(!mentions_missing_channel); // safety: test-only assertion
+ assert_eq!(
+ result.result.as_str(),
+ Some("Sent message to telegram:interactive-chat-user")
+ );
+ assert!(gateway_captures.lock().await.is_empty());
+ let telegram = telegram_captures.lock().await.clone();
+ assert_eq!(telegram.len(), 1);
+ assert_eq!(telegram[0].0, "interactive-chat-user");
+ assert_eq!(telegram[0].1.content, "NEAR price is $5");
}
#[tokio::test]
diff --git a/src/worker/job.rs b/src/worker/job.rs
index 436a23ce..ba5d47b9 100644
--- a/src/worker/job.rs
+++ b/src/worker/job.rs
@@ -1438,6 +1438,9 @@ impl From for Result {
#[cfg(test)]
mod tests {
+ use std::sync::Arc;
+
+ use crate::channels::ChannelManager;
use crate::llm::ToolSelection;
use super::*;
@@ -1448,6 +1451,8 @@ mod tests {
ToolCompletionResponse,
};
use crate::safety::SafetyLayer;
+ use crate::testing::{BroadcastCapture, RecordingBroadcastChannel};
+ use crate::tools::builtin::MessageTool;
use crate::tools::{Tool, ToolError as ToolExecError, ToolOutput};
/// A test tool that sleeps for a configurable duration before returning.
@@ -1539,6 +1544,20 @@ mod tests {
Worker::new(job_id, deps)
}
+ async fn make_worker_with_message_tool()
+ -> (Worker, Arc, BroadcastCapture, BroadcastCapture) {
+ let channel_manager = ChannelManager::new();
+ let (gateway, gateway_captures) = RecordingBroadcastChannel::new("gateway");
+ let (telegram, telegram_captures) = RecordingBroadcastChannel::new("telegram");
+ channel_manager.add(Box::new(gateway)).await;
+ channel_manager.add(Box::new(telegram)).await;
+
+ let message_tool = Arc::new(MessageTool::new(Arc::new(channel_manager)));
+ let worker = make_worker(vec![message_tool.clone()]).await;
+
+ (worker, message_tool, gateway_captures, telegram_captures)
+ }
+
#[test]
fn test_tool_selection_preserves_call_id() {
let selection = ToolSelection {
@@ -2147,4 +2166,50 @@ mod tests {
assert_eq!(ctx.metadata, original); // safety: test
}
+
+ #[tokio::test]
+ async fn autonomous_message_tool_ignores_stale_gateway_context_when_routine_metadata_targets_telegram()
+ {
+ let (worker, message_tool, gateway_captures, telegram_captures) =
+ make_worker_with_message_tool().await;
+
+ message_tool
+ .set_context(
+ Some("gateway".to_string()),
+ Some("stale-gateway-target".to_string()),
+ )
+ .await;
+
+ worker
+ .context_manager()
+ .update_context(worker.job_id, |ctx| {
+ ctx.user_id = "telegram".to_string();
+ ctx.metadata = serde_json::json!({
+ "notify_channel": "telegram",
+ "owner_id": "owner-scope",
+ });
+ Ok::<(), String>(())
+ })
+ .await
+ .unwrap() // safety: test
+ .unwrap(); // safety: test
+
+ let result = worker
+ .execute_tool(
+ "message",
+ &serde_json::json!({"content": "hello from routine"}),
+ )
+ .await
+ .unwrap(); // safety: test
+ assert!(
+ result.contains("telegram:owner-scope"),
+ "expected telegram owner-scope routing, got: {result}"
+ );
+
+ assert!(gateway_captures.lock().await.is_empty());
+ let telegram = telegram_captures.lock().await.clone();
+ assert_eq!(telegram.len(), 1);
+ assert_eq!(telegram[0].0, "owner-scope");
+ assert_eq!(telegram[0].1.content, "hello from routine");
+ }
}
From 8f6999a0740a0222ecb52ddafb54084a97c75490 Mon Sep 17 00:00:00 2001
From: Vitali Avagyan
Date: Mon, 23 Mar 2026 08:03:51 +0400
Subject: [PATCH 08/31] docs: add gitcgr code graph badge (#1563)
---
README.md | 3 +++
1 file changed, 3 insertions(+)
diff --git a/README.md b/README.md
index 6e14d9ea..cb759236 100644
--- a/README.md
+++ b/README.md
@@ -12,6 +12,9 @@
+
+
+
From d9358b0fa9a551dbad13a55aeaeaee923683394f Mon Sep 17 00:00:00 2001
From: standardtoaster
Date: Mon, 23 Mar 2026 06:56:26 +0100
Subject: [PATCH 09/31] feat(workspace): multi-scope workspace reads (#1117)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* feat(workspace): multi-scope workspace reads
Adds the ability for a workspace to read from multiple user scopes
while keeping writes isolated to the primary scope. Configuration
via WORKSPACE_READ_SCOPES env var (comma-separated user IDs).
Includes identity file isolation (read_primary), multi-scope search,
list, and read operations, WorkspaceConfig refactor, and comprehensive
integration tests.
* fix: address review feedback for multi-scope workspace reads
- fix(memory): deduplicate timezone parsing for daily_log target
parse_timezone was called twice when target was "daily_log" without a
layer — once in path resolution, again in the fallback. Now computed
once and reused.
- fix(config): add character validation for WORKSPACE_READ_SCOPES and
layer scopes — both enforce [a-zA-Z0-9_-] to prevent path traversal
or injection via scope strings used as user_id in SQL queries.
- fix(config): use chars().take(32) instead of byte-index slicing for
scope length error messages (UTF-8 safety).
- fix(error): remove unused WorkspaceError::NotFound variant
Co-Authored-By: Claude Opus 4.6 (1M context)
* style: downgrade search log to debug, add comments on list iteration
- Downgrade hybrid_search_multi tracing::info! to debug! — fires on
every multi-scope search with the default backend, too noisy for info
- Add comments explaining why list/list_all iterate per-scope instead
of using _multi trait methods (identity path filtering needs scope
attribution that merged results lose)
Co-Authored-By: Claude Opus 4.6 (1M context)
---------
Co-authored-by: ilblackdragon@gmail.com
Co-authored-by: Claude Opus 4.6 (1M context)
---
src/app.rs | 11 +
src/channels/web/server.rs | 8 +-
src/config/mod.rs | 15 +-
src/config/workspace.rs | 75 ++++-
src/db/mod.rs | 97 +++++++
src/db/postgres.rs | 45 +++
src/error.rs | 3 -
src/tools/builtin/memory.rs | 7 +-
src/workspace/README.md | 21 ++
src/workspace/document.rs | 171 ++++++++++-
src/workspace/mod.rs | 400 +++++++++++++++++++++++---
src/workspace/repository.rs | 199 +++++++++++++
tests/identity_scope_isolation.rs | 195 +++++++++++++
tests/multi_scope_functional.rs | 451 ++++++++++++++++++++++++++++++
tests/workspace_integration.rs | 330 ++++++++++++++++++++++
15 files changed, 1964 insertions(+), 64 deletions(-)
create mode 100644 tests/identity_scope_isolation.rs
create mode 100644 tests/multi_scope_functional.rs
diff --git a/src/app.rs b/src/app.rs
index b2520144..94d949be 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -325,9 +325,20 @@ impl AppBuilder {
};
let mut ws = Workspace::new_with_db(workspace_user_id, db.clone())
.with_search_config(&self.config.search);
+
if let Some(ref emb) = embeddings {
ws = ws.with_embeddings_cached(emb.clone(), emb_cache_config);
}
+
+ // Wire workspace-level settings (read scopes, memory layers)
+ if !self.config.workspace.read_scopes.is_empty() {
+ ws = ws.with_additional_read_scopes(self.config.workspace.read_scopes.clone());
+ tracing::info!(
+ user_id = workspace_user_id,
+ read_scopes = ?ws.read_user_ids(),
+ "Workspace configured with multi-scope reads"
+ );
+ }
ws = ws.with_memory_layers(self.config.workspace.memory_layers.clone());
let ws = Arc::new(ws);
tools.register_memory_tools(Arc::clone(&ws));
diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs
index 7b24805c..7edaad67 100644
--- a/src/channels/web/server.rs
+++ b/src/channels/web/server.rs
@@ -1822,7 +1822,13 @@ async fn memory_write_handler(
"Workspace not available".to_string(),
))?;
- // Route through layer-aware methods when a layer is specified
+ // Route through layer-aware methods when a layer is specified.
+ //
+ // Note: unlike MemoryWriteTool, this endpoint does NOT block writes to
+ // identity files (IDENTITY.md, SOUL.md, etc.). The HTTP API is an
+ // authenticated admin interface; the supervisor uses it to seed identity
+ // files at startup. Identity-file protection is enforced at the tool
+ // layer (LLM-facing) where the write originates from an untrusted agent.
if let Some(ref layer_name) = req.layer {
let result = if req.append {
workspace
diff --git a/src/config/mod.rs b/src/config/mod.rs
index 68b23ab2..dcda0fe9 100644
--- a/src/config/mod.rs
+++ b/src/config/mod.rs
@@ -24,7 +24,7 @@ mod skills;
mod transcription;
mod tunnel;
mod wasm;
-mod workspace;
+pub(crate) mod workspace;
use std::collections::HashMap;
use std::sync::{LazyLock, Mutex, Once};
@@ -178,9 +178,7 @@ impl Config {
},
transcription: TranscriptionConfig::default(),
search: WorkspaceSearchConfig::default(),
- workspace: WorkspaceConfig {
- memory_layers: vec![],
- },
+ workspace: WorkspaceConfig::default(),
observability: crate::observability::ObservabilityConfig::default(),
relay: None,
}
@@ -313,11 +311,14 @@ impl Config {
let tunnel = TunnelConfig::resolve(settings)?;
let channels = ChannelsConfig::resolve(settings, &owner_id)?;
+
+ // Resolve workspace config using the gateway user_id for default layers.
let workspace_user_id = channels
.gateway
.as_ref()
- .map(|gw| gw.user_id.clone())
- .unwrap_or_else(|| "default".to_string());
+ .map(|gw| gw.user_id.as_str())
+ .unwrap_or("default");
+ let workspace = WorkspaceConfig::resolve(workspace_user_id)?;
Ok(Self {
owner_id: owner_id.clone(),
@@ -339,7 +340,7 @@ impl Config {
skills: SkillsConfig::resolve()?,
transcription: TranscriptionConfig::resolve(settings)?,
search: WorkspaceSearchConfig::resolve()?,
- workspace: WorkspaceConfig::resolve(&workspace_user_id)?,
+ workspace,
observability: crate::observability::ObservabilityConfig {
backend: std::env::var("OBSERVABILITY_BACKEND").unwrap_or_else(|_| "none".into()),
},
diff --git a/src/config/workspace.rs b/src/config/workspace.rs
index 5daa73eb..27bc06f0 100644
--- a/src/config/workspace.rs
+++ b/src/config/workspace.rs
@@ -2,18 +2,29 @@ use crate::config::helpers::optional_env;
use crate::error::ConfigError;
use crate::workspace::layer::MemoryLayer;
-/// Workspace memory configuration.
+/// Workspace-level configuration (memory layers, read scopes).
///
-/// Controls memory layer definitions for privacy-aware writes.
-/// Layers are parsed from the `MEMORY_LAYERS` env var (JSON array)
-/// or default to a single private layer scoped to the gateway user.
-#[derive(Debug, Clone)]
+/// Parsed from environment variables. Lives outside of `GatewayConfig`
+/// so that non-gateway channels can eventually use the same settings.
+#[derive(Debug, Clone, Default)]
pub struct WorkspaceConfig {
+ /// Memory layer definitions (JSON in `MEMORY_LAYERS` env var, or defaults).
pub memory_layers: Vec,
+ /// Additional user scopes for workspace reads.
+ ///
+ /// When set, the workspace can read (search, read, list) from these
+ /// additional user scopes while writes remain isolated to the primary
+ /// `user_id`. Parsed from `WORKSPACE_READ_SCOPES` (comma-separated).
+ pub read_scopes: Vec,
}
impl WorkspaceConfig {
- pub(crate) fn resolve(user_id: &str) -> Result {
+ /// Resolve workspace config from environment variables.
+ ///
+ /// `user_id` is used to derive default memory layers when `MEMORY_LAYERS`
+ /// is not set.
+ pub fn resolve(user_id: &str) -> Result {
+ // --- Memory layers ---
let memory_layers: Vec = match optional_env("MEMORY_LAYERS")? {
Some(json_str) => {
serde_json::from_str(&json_str).map_err(|e| ConfigError::InvalidValue {
@@ -57,6 +68,20 @@ impl WorkspaceConfig {
message: format!("layer '{}' has an empty scope", layer.name),
});
}
+ if !layer
+ .scope
+ .chars()
+ .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
+ {
+ return Err(ConfigError::InvalidValue {
+ key: "MEMORY_LAYERS".to_string(),
+ message: format!(
+ "layer '{}' scope '{}' contains invalid characters \
+ (allowed: a-z, A-Z, 0-9, _, -)",
+ layer.name, layer.scope
+ ),
+ });
+ }
}
// Check for duplicate layer names
@@ -72,7 +97,43 @@ impl WorkspaceConfig {
}
}
- Ok(Self { memory_layers })
+ // --- Read scopes ---
+ let read_scopes: Vec = optional_env("WORKSPACE_READ_SCOPES")?
+ .map(|s| {
+ s.split(',')
+ .map(|s| s.trim().to_string())
+ .filter(|s| !s.is_empty())
+ .collect()
+ })
+ .unwrap_or_default();
+
+ for scope in &read_scopes {
+ if scope.len() > 128 {
+ let prefix: String = scope.chars().take(32).collect();
+ return Err(ConfigError::InvalidValue {
+ key: "WORKSPACE_READ_SCOPES".to_string(),
+ message: format!("scope '{prefix}...' exceeds 128 characters"),
+ });
+ }
+ if !scope
+ .chars()
+ .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
+ {
+ return Err(ConfigError::InvalidValue {
+ key: "WORKSPACE_READ_SCOPES".to_string(),
+ message: format!(
+ "scope '{}' contains invalid characters \
+ (allowed: a-z, A-Z, 0-9, _, -)",
+ scope
+ ),
+ });
+ }
+ }
+
+ Ok(Self {
+ memory_layers,
+ read_scopes,
+ })
}
}
diff --git a/src/db/mod.rs b/src/db/mod.rs
index 900d1810..0c84d35d 100644
--- a/src/db/mod.rs
+++ b/src/db/mod.rs
@@ -644,6 +644,103 @@ pub trait WorkspaceStore: Send + Sync {
embedding: Option<&[f32]>,
config: &SearchConfig,
) -> Result, WorkspaceError>;
+
+ // ==================== Multi-scope read methods ====================
+ //
+ // Default implementations loop over user_ids calling single-scope methods,
+ // then merge results. Backends can override with efficient SQL (e.g.,
+ // `WHERE user_id = ANY($1::text[])`).
+
+ /// Hybrid search across multiple user scopes, merging results by score.
+ ///
+ /// **Note:** The default implementation calls `hybrid_search` per scope and
+ /// merges by raw score. Because RRF scores are normalized independently
+ /// within each scope, scores are not directly comparable across scopes.
+ /// The Postgres backend overrides this with a single combined query that
+ /// applies RRF once to the unified result set.
+ async fn hybrid_search_multi(
+ &self,
+ user_ids: &[String],
+ agent_id: Option,
+ query: &str,
+ embedding: Option<&[f32]>,
+ config: &SearchConfig,
+ ) -> Result, WorkspaceError> {
+ if user_ids.len() > 1 {
+ tracing::debug!(
+ scope_count = user_ids.len(),
+ "hybrid_search_multi: using default per-scope RRF merge; \
+ cross-scope score comparison may be unreliable"
+ );
+ }
+ let mut all_results = Vec::new();
+ for uid in user_ids {
+ let results = self
+ .hybrid_search(uid, agent_id, query, embedding, config)
+ .await?;
+ all_results.extend(results);
+ }
+ // Re-sort by score descending and truncate to limit
+ all_results.sort_by(|a, b| {
+ b.score
+ .partial_cmp(&a.score)
+ .unwrap_or(std::cmp::Ordering::Equal)
+ });
+ all_results.truncate(config.limit);
+ Ok(all_results)
+ }
+
+ /// List all file paths across multiple user scopes.
+ async fn list_all_paths_multi(
+ &self,
+ user_ids: &[String],
+ agent_id: Option,
+ ) -> Result, WorkspaceError> {
+ let mut all_paths = Vec::new();
+ for uid in user_ids {
+ let paths = self.list_all_paths(uid, agent_id).await?;
+ all_paths.extend(paths);
+ }
+ all_paths.sort();
+ all_paths.dedup();
+ Ok(all_paths)
+ }
+
+ /// Get a document by path, searching across multiple user scopes.
+ ///
+ /// Returns the first match found (tries each user_id in order).
+ async fn get_document_by_path_multi(
+ &self,
+ user_ids: &[String],
+ agent_id: Option,
+ path: &str,
+ ) -> Result {
+ for uid in user_ids {
+ match self.get_document_by_path(uid, agent_id, path).await {
+ Ok(doc) => return Ok(doc),
+ Err(WorkspaceError::DocumentNotFound { .. }) => continue,
+ Err(e) => return Err(e),
+ }
+ }
+ Err(WorkspaceError::DocumentNotFound {
+ doc_type: path.to_string(),
+ user_id: format!("[{}]", user_ids.join(", ")),
+ })
+ }
+
+ /// List directory contents across multiple user scopes.
+ async fn list_directory_multi(
+ &self,
+ user_ids: &[String],
+ agent_id: Option,
+ directory: &str,
+ ) -> Result, WorkspaceError> {
+ let mut all_entries = Vec::new();
+ for uid in user_ids {
+ all_entries.extend(self.list_directory(uid, agent_id, directory).await?);
+ }
+ Ok(crate::workspace::merge_workspace_entries(all_entries))
+ }
}
/// Backend-agnostic database supertrait.
diff --git a/src/db/postgres.rs b/src/db/postgres.rs
index e77452db..cfa10997 100644
--- a/src/db/postgres.rs
+++ b/src/db/postgres.rs
@@ -717,4 +717,49 @@ impl WorkspaceStore for PgBackend {
.hybrid_search(user_id, agent_id, query, embedding, config)
.await
}
+
+ // Optimized multi-scope overrides using `ANY($1::text[])` SQL.
+
+ async fn hybrid_search_multi(
+ &self,
+ user_ids: &[String],
+ agent_id: Option,
+ query: &str,
+ embedding: Option<&[f32]>,
+ config: &SearchConfig,
+ ) -> Result, WorkspaceError> {
+ self.repo
+ .hybrid_search_multi(user_ids, agent_id, query, embedding, config)
+ .await
+ }
+
+ async fn list_all_paths_multi(
+ &self,
+ user_ids: &[String],
+ agent_id: Option,
+ ) -> Result, WorkspaceError> {
+ self.repo.list_all_paths_multi(user_ids, agent_id).await
+ }
+
+ async fn get_document_by_path_multi(
+ &self,
+ user_ids: &[String],
+ agent_id: Option,
+ path: &str,
+ ) -> Result {
+ self.repo
+ .get_document_by_path_multi(user_ids, agent_id, path)
+ .await
+ }
+
+ async fn list_directory_multi(
+ &self,
+ user_ids: &[String],
+ agent_id: Option,
+ directory: &str,
+ ) -> Result, WorkspaceError> {
+ self.repo
+ .list_directory_multi(user_ids, agent_id, directory)
+ .await
+ }
}
diff --git a/src/error.rs b/src/error.rs
index 30ec58f4..e4f1b957 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -304,9 +304,6 @@ pub enum WorkspaceError {
#[error("I/O error: {reason}")]
IoError { reason: String },
- #[error("Not found: {path}")]
- NotFound { path: String },
-
#[error("Layer not found: {name}")]
LayerNotFound { name: String },
diff --git a/src/tools/builtin/memory.rs b/src/tools/builtin/memory.rs
index 1c27b539..edbc4f1c 100644
--- a/src/tools/builtin/memory.rs
+++ b/src/tools/builtin/memory.rs
@@ -271,12 +271,13 @@ impl Tool for MemoryWriteTool {
.and_then(|v| v.as_bool())
.unwrap_or(false);
+ // Parse timezone once for targets that need it (daily_log).
+ let tz = crate::timezone::parse_timezone(&ctx.user_timezone).unwrap_or(chrono_tz::Tz::UTC);
+
// Resolve the target to a workspace path
let resolved_path = match target {
"memory" => paths::MEMORY.to_string(),
"daily_log" => {
- let tz = crate::timezone::parse_timezone(&ctx.user_timezone)
- .unwrap_or(chrono_tz::Tz::UTC);
let now = chrono::Utc::now().with_timezone(&tz);
format!("daily/{}.md", now.format("%Y-%m-%d"))
}
@@ -318,8 +319,6 @@ impl Tool for MemoryWriteTool {
}
}
"daily_log" => {
- let tz = crate::timezone::parse_timezone(&ctx.user_timezone)
- .unwrap_or(chrono_tz::Tz::UTC);
self.workspace
.append_daily_log_tz(content, tz)
.await
diff --git a/src/workspace/README.md b/src/workspace/README.md
index 67b9907f..061a5564 100644
--- a/src/workspace/README.md
+++ b/src/workspace/README.md
@@ -91,6 +91,27 @@ Default k=60. Results from both methods are combined, with documents appearing i
- **PostgreSQL:** `ts_rank_cd` for FTS, pgvector cosine distance for vectors, full RRF
- **libSQL:** FTS5 for keyword search + vector search via `libsql_vector_idx` (dimension set dynamically by `ensure_vector_index()` during startup)
+## Multi-Scope Reads & Identity Isolation
+
+When a workspace has additional read scopes (via `with_additional_read_scopes`), read operations can span multiple user scopes — a user with scopes `["alice", "shared"]` can read documents from both.
+
+**Identity files are exempt from multi-scope reads.** The system prompt reads identity and configuration files from the **primary scope only** (`read_primary()`), never from secondary scopes:
+
+| File | Read method | Rationale |
+|------|------------|-----------|
+| AGENTS.md | `read_primary()` | Agent instructions are per-user |
+| SOUL.md | `read_primary()` | Core values are per-user |
+| USER.md | `read_primary()` | User context is per-user |
+| IDENTITY.md | `read_primary()` | Identity is per-user |
+| TOOLS.md | `read_primary()` | Tool config is per-user |
+| BOOTSTRAP.md | `read_primary()` | Onboarding is per-user |
+| MEMORY.md | `read()` | Shared memory is a feature |
+| daily/*.md | `read()` | Shared daily logs are a feature |
+
+**Why:** Without this, a user with read access to another scope could silently inherit that scope's identity if their own copy is missing. The agent would present itself as the wrong user — a correctness and security issue.
+
+**Design rule:** If you want shared identity across users, seed the same content into each user's scope at setup time. Don't rely on multi-scope fallback for identity files.
+
## Heartbeat System
Proactive periodic execution (default: 30 minutes):
diff --git a/src/workspace/document.rs b/src/workspace/document.rs
index 3396b677..b1fa176a 100644
--- a/src/workspace/document.rs
+++ b/src/workspace/document.rs
@@ -37,6 +37,25 @@ pub mod paths {
pub const ASSISTANT_DIRECTIVES: &str = "context/assistant-directives.md";
}
+/// Paths treated as identity documents for multi-scope isolation.
+///
+/// These files are always read from the primary scope only — never from
+/// secondary read scopes. This prevents silent identity inheritance
+/// (e.g., user A accidentally presenting as user B).
+pub const IDENTITY_PATHS: &[&str] = &[
+ paths::IDENTITY,
+ paths::SOUL,
+ paths::AGENTS,
+ paths::USER,
+ paths::TOOLS,
+ paths::BOOTSTRAP,
+];
+
+/// Check if a path is an identity document that must be isolated to primary scope.
+pub fn is_identity_path(path: &str) -> bool {
+ IDENTITY_PATHS.contains(&path)
+}
+
/// A memory document stored in the database.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct MemoryDocument {
@@ -101,10 +120,7 @@ impl MemoryDocument {
/// Check if this is a well-known identity document.
pub fn is_identity_document(&self) -> bool {
- matches!(
- self.path.as_str(),
- paths::IDENTITY | paths::SOUL | paths::AGENTS | paths::USER
- )
+ is_identity_path(&self.path)
}
}
@@ -128,6 +144,42 @@ impl WorkspaceEntry {
}
}
+/// Merge workspace entries from multiple scopes into a deduplicated, sorted list.
+///
+/// When the same path appears in multiple scopes:
+/// - Keeps the most recent `updated_at`
+/// - If any scope marks it as a directory, the merged entry is a directory
+pub fn merge_workspace_entries(
+ entries: impl IntoIterator- ,
+) -> Vec {
+ let mut seen = std::collections::HashMap::new();
+ for entry in entries {
+ seen.entry(entry.path.clone())
+ .and_modify(|existing: &mut WorkspaceEntry| {
+ // Keep the most recent updated_at (and its content_preview)
+ if let (Some(existing_ts), Some(new_ts)) = (&existing.updated_at, &entry.updated_at)
+ {
+ if new_ts > existing_ts {
+ existing.updated_at = Some(*new_ts);
+ existing.content_preview = entry.content_preview.clone();
+ }
+ } else if existing.updated_at.is_none() {
+ existing.updated_at = entry.updated_at;
+ existing.content_preview = entry.content_preview.clone();
+ }
+ // If either is a directory, mark as directory
+ if entry.is_directory {
+ existing.is_directory = true;
+ existing.content_preview = None;
+ }
+ })
+ .or_insert(entry);
+ }
+ let mut result: Vec = seen.into_values().collect();
+ result.sort_by(|a, b| a.path.cmp(&b.path));
+ result
+}
+
/// A chunk of a memory document for search indexing.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct MemoryChunk {
@@ -226,4 +278,115 @@ mod tests {
};
assert_eq!(entry.name(), "alpha");
}
+
+ #[test]
+ fn test_merge_workspace_entries_empty() {
+ let result = merge_workspace_entries(vec![]);
+ assert!(result.is_empty());
+ }
+
+ #[test]
+ fn test_merge_workspace_entries_keeps_newer_timestamp_and_preview() {
+ use chrono::TimeZone;
+ let old_ts = chrono::Utc.with_ymd_and_hms(2025, 1, 1, 0, 0, 0).unwrap();
+ let new_ts = chrono::Utc.with_ymd_and_hms(2025, 6, 1, 0, 0, 0).unwrap();
+
+ let entries = vec![
+ WorkspaceEntry {
+ path: "notes.md".to_string(),
+ is_directory: false,
+ updated_at: Some(old_ts),
+ content_preview: Some("old".to_string()),
+ },
+ WorkspaceEntry {
+ path: "notes.md".to_string(),
+ is_directory: false,
+ updated_at: Some(new_ts),
+ content_preview: Some("new".to_string()),
+ },
+ ];
+
+ let result = merge_workspace_entries(entries);
+ assert_eq!(result.len(), 1);
+ assert_eq!(result[0].updated_at, Some(new_ts));
+ assert_eq!(result[0].content_preview, Some("new".to_string()));
+ }
+
+ #[test]
+ fn test_merge_workspace_entries_directory_wins() {
+ let entries = vec![
+ WorkspaceEntry {
+ path: "projects".to_string(),
+ is_directory: false,
+ updated_at: None,
+ content_preview: Some("file content".to_string()),
+ },
+ WorkspaceEntry {
+ path: "projects".to_string(),
+ is_directory: true,
+ updated_at: None,
+ content_preview: None,
+ },
+ ];
+
+ let result = merge_workspace_entries(entries);
+ assert_eq!(result.len(), 1);
+ assert!(result[0].is_directory);
+ assert!(result[0].content_preview.is_none());
+ }
+
+ #[test]
+ fn test_merge_workspace_entries_fills_missing_timestamp() {
+ use chrono::TimeZone;
+ let ts = chrono::Utc.with_ymd_and_hms(2025, 3, 1, 0, 0, 0).unwrap();
+
+ let entries = vec![
+ WorkspaceEntry {
+ path: "a.md".to_string(),
+ is_directory: false,
+ updated_at: None,
+ content_preview: None,
+ },
+ WorkspaceEntry {
+ path: "a.md".to_string(),
+ is_directory: false,
+ updated_at: Some(ts),
+ content_preview: None,
+ },
+ ];
+
+ let result = merge_workspace_entries(entries);
+ assert_eq!(result.len(), 1);
+ assert_eq!(result[0].updated_at, Some(ts));
+ }
+
+ #[test]
+ fn test_merge_workspace_entries_sorted_by_path() {
+ let entries = vec![
+ WorkspaceEntry {
+ path: "z.md".to_string(),
+ is_directory: false,
+ updated_at: None,
+ content_preview: None,
+ },
+ WorkspaceEntry {
+ path: "a.md".to_string(),
+ is_directory: false,
+ updated_at: None,
+ content_preview: None,
+ },
+ WorkspaceEntry {
+ path: "m.md".to_string(),
+ is_directory: false,
+ updated_at: None,
+ content_preview: None,
+ },
+ ];
+
+ let result = merge_workspace_entries(entries);
+ assert_eq!(result.len(), 3);
+ assert_eq!(result[0].path, "a.md");
+ assert_eq!(result[1].path, "m.md");
+ assert_eq!(result[2].path, "z.md");
+ }
}
diff --git a/src/workspace/mod.rs b/src/workspace/mod.rs
index 5aac2500..0242047f 100644
--- a/src/workspace/mod.rs
+++ b/src/workspace/mod.rs
@@ -52,7 +52,10 @@ mod repository;
mod search;
pub use chunker::{ChunkConfig, chunk_document};
-pub use document::{MemoryChunk, MemoryDocument, WorkspaceEntry, paths};
+pub use document::{
+ IDENTITY_PATHS, MemoryChunk, MemoryDocument, WorkspaceEntry, is_identity_path,
+ merge_workspace_entries, paths,
+};
pub use embedding_cache::{CachedEmbeddingProvider, EmbeddingCacheConfig};
pub use embeddings::{
EmbeddingProvider, MockEmbeddings, NearAiEmbeddings, OllamaEmbeddings, OpenAiEmbeddings,
@@ -320,6 +323,48 @@ impl WorkspaceStorage {
}
}
}
+
+ // ==================== Multi-scope read methods ====================
+
+ async fn hybrid_search_multi(
+ &self,
+ user_ids: &[String],
+ agent_id: Option,
+ query: &str,
+ embedding: Option<&[f32]>,
+ config: &SearchConfig,
+ ) -> Result, WorkspaceError> {
+ match self {
+ #[cfg(feature = "postgres")]
+ Self::Repo(repo) => {
+ repo.hybrid_search_multi(user_ids, agent_id, query, embedding, config)
+ .await
+ }
+ Self::Db(db) => {
+ db.hybrid_search_multi(user_ids, agent_id, query, embedding, config)
+ .await
+ }
+ }
+ }
+
+ async fn get_document_by_path_multi(
+ &self,
+ user_ids: &[String],
+ agent_id: Option,
+ path: &str,
+ ) -> Result {
+ match self {
+ #[cfg(feature = "postgres")]
+ Self::Repo(repo) => {
+ repo.get_document_by_path_multi(user_ids, agent_id, path)
+ .await
+ }
+ Self::Db(db) => {
+ db.get_document_by_path_multi(user_ids, agent_id, path)
+ .await
+ }
+ }
+ }
}
/// Default template seeded into HEARTBEAT.md on first access.
@@ -340,9 +385,20 @@ const BOOTSTRAP_SEED: &str = include_str!("seeds/BOOTSTRAP.md");
/// Each workspace is scoped to a user (and optionally an agent).
/// Documents are persisted to the database and indexed for search.
/// Supports both PostgreSQL (via Repository) and libSQL (via Database trait).
+///
+/// ## Multi-scope reads
+///
+/// By default, a workspace reads from and writes to a single `user_id`.
+/// With `with_additional_read_scopes`, read operations (search, read, list)
+/// can span multiple user scopes while writes remain isolated to the primary
+/// `user_id`. This enables cross-tenant read access (e.g., a user reading
+/// from both their own workspace and a "shared" workspace).
pub struct Workspace {
- /// User identifier (from channel).
+ /// User identifier (from channel). All writes go to this scope.
user_id: String,
+ /// User identifiers for read operations. Includes `user_id` as the first
+ /// element, plus any additional scopes added via `with_additional_read_scopes`.
+ read_user_ids: Vec,
/// Optional agent ID for multi-agent isolation.
agent_id: Option,
/// Database storage backend.
@@ -371,6 +427,7 @@ impl Workspace {
let user_id_str = user_id.into();
let memory_layers = crate::workspace::layer::MemoryLayer::default_for_user(&user_id_str);
Self {
+ read_user_ids: vec![user_id_str.clone()],
user_id: user_id_str,
agent_id: None,
storage: WorkspaceStorage::Repo(Repository::new(pool)),
@@ -390,6 +447,7 @@ impl Workspace {
let user_id_str = user_id.into();
let memory_layers = crate::workspace::layer::MemoryLayer::default_for_user(&user_id_str);
Self {
+ read_user_ids: vec![user_id_str.clone()],
user_id: user_id_str,
agent_id: None,
storage: WorkspaceStorage::Db(db),
@@ -474,6 +532,12 @@ impl Workspace {
///
/// Also updates read_user_ids to include all layer scopes.
pub fn with_memory_layers(mut self, layers: Vec) -> Self {
+ // Add layer scopes to read_user_ids (same dedup logic as with_additional_read_scopes)
+ for layer in &layers {
+ if !self.read_user_ids.contains(&layer.scope) {
+ self.read_user_ids.push(layer.scope.clone());
+ }
+ }
self.memory_layers = layers;
self
}
@@ -496,11 +560,37 @@ impl Workspace {
&self.memory_layers
}
- /// Get the user ID.
+ /// Add additional user scopes for read operations.
+ ///
+ /// The primary `user_id` is always included. Additional scopes allow
+ /// read operations (search, read, list) to span multiple tenants while
+ /// writes remain isolated to the primary scope.
+ ///
+ /// Duplicate scopes are ignored.
+ pub fn with_additional_read_scopes(mut self, scopes: Vec) -> Self {
+ for scope in scopes {
+ if !self.read_user_ids.contains(&scope) {
+ self.read_user_ids.push(scope);
+ }
+ }
+ self
+ }
+
+ /// Get the user ID (primary scope for writes).
pub fn user_id(&self) -> &str {
&self.user_id
}
+ /// Get the user IDs used for read operations.
+ pub fn read_user_ids(&self) -> &[String] {
+ &self.read_user_ids
+ }
+
+ /// Whether this workspace has multiple read scopes.
+ fn is_multi_scope(&self) -> bool {
+ self.read_user_ids.len() > 1
+ }
+
/// Get the agent ID.
pub fn agent_id(&self) -> Option {
self.agent_id
@@ -518,6 +608,33 @@ impl Workspace {
/// println!("{}", doc.content);
/// ```
pub async fn read(&self, path: &str) -> Result {
+ let path = normalize_path(path);
+ if self.is_multi_scope() && is_identity_path(&path) {
+ // Identity files must only come from the primary scope.
+ self.storage
+ .get_document_by_path(&self.user_id, self.agent_id, &path)
+ .await
+ } else if self.is_multi_scope() {
+ self.storage
+ .get_document_by_path_multi(&self.read_user_ids, self.agent_id, &path)
+ .await
+ } else {
+ self.storage
+ .get_document_by_path(&self.user_id, self.agent_id, &path)
+ .await
+ }
+ }
+
+ /// Read a file from the **primary scope only**, ignoring additional read scopes.
+ ///
+ /// Use this for identity and configuration files (AGENTS.md, SOUL.md, USER.md,
+ /// IDENTITY.md, TOOLS.md, BOOTSTRAP.md) where inheriting content from another
+ /// scope would be a correctness/security issue — the agent must never silently
+ /// present itself as the wrong user.
+ ///
+ /// For memory files that should span scopes (MEMORY.md, daily logs), use
+ /// [`read`] instead.
+ pub async fn read_primary(&self, path: &str) -> Result {
let path = normalize_path(path);
self.storage
.get_document_by_path(&self.user_id, self.agent_id, &path)
@@ -556,6 +673,9 @@ impl Workspace {
/// Uses a single `\n` separator (suitable for log-style entries).
/// For semantic separation (e.g., memory entries), use `append_memory()`
/// which uses `\n\n`.
+ ///
+ /// Uses a read-modify-write pattern that is not concurrency-safe:
+ /// concurrent appends to the same path may lose writes.
pub async fn append(&self, path: &str, content: &str) -> Result<(), WorkspaceError> {
let path = normalize_path(path);
// Scan system-prompt-injected files for prompt injection.
@@ -676,6 +796,20 @@ impl Workspace {
}
/// Write to a layer, with append semantics.
+ ///
+ /// Note: privacy classification only examines the new `content`, not the
+ /// full document after concatenation. See [`PatternPrivacyClassifier`]
+ /// limitations for details.
+ ///
+ /// When a privacy redirect occurs, the append targets a **separate
+ /// document** in the private scope at the same path — the shared-scope
+ /// document is left unmodified. Subsequent multi-scope reads will return
+ /// the private copy (primary scope wins), effectively shadowing the
+ /// shared document at that path. The `WriteResult::redirected` flag
+ /// indicates when this has happened.
+ ///
+ /// Uses a read-modify-write pattern that is not concurrency-safe:
+ /// concurrent appends to the same path may lose writes.
pub async fn append_to_layer(
&self,
layer_name: &str,
@@ -706,13 +840,25 @@ impl Workspace {
}
/// Check if a file exists.
+ ///
+ /// When multi-scope reads are configured, checks across all read scopes.
pub async fn exists(&self, path: &str) -> Result {
let path = normalize_path(path);
- match self
- .storage
- .get_document_by_path(&self.user_id, self.agent_id, &path)
- .await
- {
+ let result = if self.is_multi_scope() && is_identity_path(&path) {
+ // Identity files only checked in primary scope.
+ self.storage
+ .get_document_by_path(&self.user_id, self.agent_id, &path)
+ .await
+ } else if self.is_multi_scope() {
+ self.storage
+ .get_document_by_path_multi(&self.read_user_ids, self.agent_id, &path)
+ .await
+ } else {
+ self.storage
+ .get_document_by_path(&self.user_id, self.agent_id, &path)
+ .await
+ };
+ match result {
Ok(_) => Ok(true),
Err(WorkspaceError::DocumentNotFound { .. }) => Ok(false),
Err(e) => Err(e),
@@ -747,16 +893,55 @@ impl Workspace {
/// ```
pub async fn list(&self, directory: &str) -> Result, WorkspaceError> {
let directory = normalize_directory(directory);
- self.storage
- .list_directory(&self.user_id, self.agent_id, &directory)
- .await
+ if self.is_multi_scope() {
+ // Iterate per-scope rather than using list_directory_multi because
+ // we need to filter identity paths from secondary scopes only — the
+ // merged _multi result loses scope attribution.
+ let primary = self
+ .storage
+ .list_directory(&self.user_id, self.agent_id, &directory)
+ .await?;
+ let mut all_entries = primary;
+ for scope in &self.read_user_ids[1..] {
+ let entries = self
+ .storage
+ .list_directory(scope, self.agent_id, &directory)
+ .await?;
+ all_entries.extend(entries.into_iter().filter(|e| !is_identity_path(&e.path)));
+ }
+ Ok(merge_workspace_entries(all_entries))
+ } else {
+ self.storage
+ .list_directory(&self.user_id, self.agent_id, &directory)
+ .await
+ }
}
/// List all files recursively (flat list of all paths).
+ ///
+ /// When multi-scope reads are configured, lists across all read scopes.
pub async fn list_all(&self) -> Result, WorkspaceError> {
- self.storage
- .list_all_paths(&self.user_id, self.agent_id)
- .await
+ if self.is_multi_scope() {
+ // Iterate per-scope rather than using list_all_paths_multi because
+ // we need to filter identity paths from secondary scopes only.
+ // Primary scope: all paths. Secondary scopes: filter identity paths.
+ let mut all_paths = self
+ .storage
+ .list_all_paths(&self.user_id, self.agent_id)
+ .await?;
+ for scope in &self.read_user_ids[1..] {
+ let paths = self.storage.list_all_paths(scope, self.agent_id).await?;
+ all_paths.extend(paths.into_iter().filter(|p| !is_identity_path(p)));
+ }
+ // Deduplicate and sort
+ all_paths.sort();
+ all_paths.dedup();
+ Ok(all_paths)
+ } else {
+ self.storage
+ .list_all_paths(&self.user_id, self.agent_id)
+ .await
+ }
}
// ==================== Convenience Methods ====================
@@ -791,7 +976,7 @@ impl Workspace {
/// comments, which the heartbeat runner treats as "effectively empty"
/// and skips the LLM call.
pub async fn heartbeat_checklist(&self) -> Result