Compare commits

..
Author SHA1 Message Date
ZakiandClaude b50f80cbda fix(agent): clarify hydration return type and pruned-thread error message (#1487)
Introduce HydrationResult enum (Ready/Skipped/NotFound) to disambiguate
the return contract of maybe_hydrate_thread — callers can now distinguish
between a fully hydrated thread and one where hydration was skipped.

Update the error message when a thread disappears during approval to
acknowledge that actions may have partially executed, rather than
suggesting a simple retry.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-23 02:11:34 +00:00
Claude fd6e1d8b1f fix(agent): match on Result return type from maybe_hydrate_thread
The maybe_hydrate_thread signature changed from Option<String> to
Result<Option<Uuid>, String> but the caller still pattern-matched
with Some(), causing a type mismatch clippy/compile error. Switch
to Err() to match the new error-variant semantics.

[skip-regression-check]

https://claude.ai/code/session_013ZCQWoFHv2hASgHEGHzptg
2026-03-23 02:11:34 +00:00
ZakiandClaude f7fbbc229b fix(agent): surface errors when approval thread disappears (#1487)
Replace silent `if let Some` fallbacks with explicit `match` arms that
log and return errors when a thread is missing from the session during
approval storage or rejection persistence.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-23 02:11:34 +00:00
33 changed files with 376 additions and 3432 deletions
+1 -1
View File
@@ -54,7 +54,7 @@ jobs:
- group: features
files: "tests/e2e/scenarios/test_skills.py tests/e2e/scenarios/test_tool_approval.py tests/e2e/scenarios/test_webhook.py"
- group: extensions
files: "tests/e2e/scenarios/test_extensions.py tests/e2e/scenarios/test_extension_oauth.py tests/e2e/scenarios/test_oauth_url_parameters.py tests/e2e/scenarios/test_telegram_token_validation.py tests/e2e/scenarios/test_telegram_hot_activation.py tests/e2e/scenarios/test_wasm_lifecycle.py tests/e2e/scenarios/test_tool_execution.py tests/e2e/scenarios/test_pairing.py tests/e2e/scenarios/test_mcp_auth_flow.py tests/e2e/scenarios/test_oauth_credential_fallback.py tests/e2e/scenarios/test_routine_oauth_credential_injection.py"
files: "tests/e2e/scenarios/test_extensions.py tests/e2e/scenarios/test_extension_oauth.py tests/e2e/scenarios/test_telegram_token_validation.py tests/e2e/scenarios/test_telegram_hot_activation.py tests/e2e/scenarios/test_wasm_lifecycle.py tests/e2e/scenarios/test_tool_execution.py tests/e2e/scenarios/test_pairing.py tests/e2e/scenarios/test_mcp_auth_flow.py tests/e2e/scenarios/test_oauth_credential_fallback.py tests/e2e/scenarios/test_routine_oauth_credential_injection.py"
- group: routines
files: "tests/e2e/scenarios/test_owner_scope.py tests/e2e/scenarios/test_routine_event_batch.py"
steps:
Generated
+12 -12
View File
@@ -157,7 +157,7 @@ version = "1.1.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc"
dependencies = [
"windows-sys 0.60.2",
"windows-sys 0.61.2",
]
[[package]]
@@ -168,7 +168,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d"
dependencies = [
"anstyle",
"once_cell_polyfill",
"windows-sys 0.60.2",
"windows-sys 0.61.2",
]
[[package]]
@@ -2136,7 +2136,7 @@ dependencies = [
"libc",
"option-ext",
"redox_users 0.5.2",
"windows-sys 0.59.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -2323,7 +2323,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb"
dependencies = [
"libc",
"windows-sys 0.52.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -4134,7 +4134,7 @@ version = "0.50.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5"
dependencies = [
"windows-sys 0.59.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -5472,7 +5472,7 @@ dependencies = [
"errno",
"libc",
"linux-raw-sys 0.12.1",
"windows-sys 0.52.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -6154,7 +6154,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3a766e1110788c36f4fa1c2b71b387a7815aa65f88ce0229841826633d93723e"
dependencies = [
"libc",
"windows-sys 0.60.2",
"windows-sys 0.61.2",
]
[[package]]
@@ -6354,9 +6354,9 @@ checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369"
[[package]]
name = "tar"
version = "0.4.45"
version = "0.4.44"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "22692a6476a21fa75fdfc11d452fda482af402c008cdbaf3476414e122040973"
checksum = "1d863878d212c87a19c1a610eb53bb01fe12951c0501cf5a0d65f724914a667a"
dependencies = [
"filetime",
"libc",
@@ -6379,7 +6379,7 @@ dependencies = [
"getrandom 0.4.2",
"once_cell",
"rustix 1.1.4",
"windows-sys 0.52.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -7179,7 +7179,7 @@ checksum = "f2f6fb2847f6742cd76af783a2a2c49e9375d0a111c7bef6f71cd9e738c72d6e"
dependencies = [
"memoffset",
"tempfile",
"windows-sys 0.60.2",
"windows-sys 0.61.2",
]
[[package]]
@@ -8029,7 +8029,7 @@ version = "0.1.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22"
dependencies = [
"windows-sys 0.48.0",
"windows-sys 0.61.2",
]
[[package]]
+1 -1
View File
@@ -161,7 +161,7 @@ This document tracks feature parity between IronClaw (Rust implementation) and O
| `config` | ✅ | ✅ | - | Read/write config plus validate/path helpers |
| `backup` | ✅ | ❌ | P3 | Create/verify local backup archives |
| `channels` | ✅ | 🚧 | P2 | `list` implemented; `enable`/`disable`/`status` deferred pending config source unification |
| `models` | ✅ | 🚧 | P1 | `models list [<provider>]` (`--verbose`, `--json`; fetches live model list when provider specified), `models status` (`--json`), `models set <model>`, `models set-provider <provider> [--model model]` (alias normalization, config.toml + .env persistence). Remaining: `set` doesn't validate model against live list. |
| `models` | ✅ | 🚧 | - | Model selector in TUI |
| `status` | ✅ | ✅ | - | System status (enriched session details) |
| `agents` | ✅ | ❌ | P3 | Multi-agent management |
| `sessions` | ✅ | ❌ | P3 | Session listing (shows subagent models) |
-3
View File
@@ -12,9 +12,6 @@
<a href="#license"><img src="https://img.shields.io/badge/license-MIT%20OR%20Apache%202.0-blue.svg" alt="License: MIT OR Apache-2.0" /></a>
<a href="https://t.me/ironclawAI"><img src="https://img.shields.io/badge/Telegram-%40ironclawAI-26A5E4?style=flat&logo=telegram&logoColor=white" alt="Telegram: @ironclawAI" /></a>
<a href="https://www.reddit.com/r/ironclawAI/"><img src="https://img.shields.io/badge/Reddit-r%2FironclawAI-FF4500?style=flat&logo=reddit&logoColor=white" alt="Reddit: r/ironclawAI" /></a>
<a href="https://gitcgr.com/nearai/ironclaw">
<img src="https://gitcgr.com/badge/nearai/ironclaw.svg" alt="gitcgr" />
</a>
</p>
<p align="center">
+7 -2
View File
@@ -1010,8 +1010,13 @@ impl Agent {
thread_id = %external_thread_id,
"Hydrating thread from DB"
);
if let Some(rejection) = self.maybe_hydrate_thread(message, external_thread_id).await {
return Ok(Some(format!("Error: {}", rejection)));
match self.maybe_hydrate_thread(message, external_thread_id).await {
Err(rejection) => {
return Ok(Some(format!("Error: {}", rejection)));
}
Ok(_) => {
// Ready, Skipped, or NotFound — all proceed to resolve_thread
}
}
}
+36 -83
View File
@@ -102,9 +102,6 @@ impl SessionManager {
/// Resolve an external thread ID to an internal thread.
///
/// Returns the session and thread ID. Creates both if they don't exist.
///
/// Uses a single read-lock acquisition for both the key lookup and the UUID
/// adoption check to reduce contention under concurrent approval load.
pub async fn resolve_thread(
&self,
user_id: &str,
@@ -119,57 +116,51 @@ impl SessionManager {
external_thread_id: external_thread_id.map(String::from),
};
// Parse UUID once outside the lock (if applicable)
let ext_uuid = external_thread_id.and_then(|s| Uuid::parse_str(s).ok());
// Single read lock for both the key lookup and UUID adoption check
let adoptable_uuid = {
// Check if we have a mapping
{
let thread_map = self.thread_map.read().await;
// Fast path: exact key match
if let Some(&thread_id) = thread_map.get(&key) {
// Verify thread still exists in session
let sess = session.lock().await;
if sess.threads.contains_key(&thread_id) {
return (Arc::clone(&session), thread_id);
}
}
}
// UUID adoption check (still under the same read lock).
// If external_thread_id is a valid UUID not mapped elsewhere,
// it may be a thread created by chat_new_thread_handler or
// hydrated from DB that we can adopt.
if let Some(ext_uuid) = ext_uuid {
let mapped_elsewhere = thread_map.values().any(|&v| v == ext_uuid);
if !mapped_elsewhere {
Some(ext_uuid)
} else {
None
// Check if external_thread_id is itself a known thread UUID that
// exists in the session but was never registered in the thread_map
// (e.g. created by chat_new_thread_handler or hydrated from DB).
// We only adopt it if no thread_map entry maps to this UUID —
// otherwise it belongs to a different channel scope.
if let Some(ext_tid) = external_thread_id
&& let Ok(ext_uuid) = Uuid::parse_str(ext_tid)
{
let thread_map = self.thread_map.read().await;
let mapped_elsewhere = thread_map.values().any(|&v| v == ext_uuid);
drop(thread_map);
if !mapped_elsewhere {
let sess = session.lock().await;
if sess.threads.contains_key(&ext_uuid) {
drop(sess);
let mut thread_map = self.thread_map.write().await;
// Re-check after acquiring write lock to prevent race condition
// where another task mapped this UUID between our read and write.
if !thread_map.values().any(|&v| v == ext_uuid) {
thread_map.insert(key, ext_uuid);
drop(thread_map);
// Ensure undo manager exists
let mut undo_managers = self.undo_managers.write().await;
undo_managers
.entry(ext_uuid)
.or_insert_with(|| Arc::new(Mutex::new(UndoManager::new())));
return (session, ext_uuid);
}
// If it was mapped elsewhere while we were unlocked, fall through
// to create a new thread, preserving channel isolation.
}
} else {
None
}
}; // Single read lock dropped here
// If we found an adoptable UUID, verify it exists in session and acquire write lock
if let Some(ext_uuid) = adoptable_uuid {
let sess = session.lock().await;
if sess.threads.contains_key(&ext_uuid) {
drop(sess);
let mut thread_map = self.thread_map.write().await;
// Re-check after acquiring write lock to prevent race condition
// where another task mapped this UUID between our read and write.
if !thread_map.values().any(|&v| v == ext_uuid) {
thread_map.insert(key, ext_uuid);
drop(thread_map);
// Ensure undo manager exists
let mut undo_managers = self.undo_managers.write().await;
undo_managers
.entry(ext_uuid)
.or_insert_with(|| Arc::new(Mutex::new(UndoManager::new())));
return (session, ext_uuid);
}
// If mapped elsewhere while unlocked, fall through to create new thread
}
}
@@ -918,44 +909,6 @@ mod tests {
}
}
#[tokio::test]
async fn test_resolve_thread_consolidates_read_path() {
// Verify that resolve_thread still correctly handles:
// 1. Fast path: key exists in thread_map
// 2. UUID adoption: external_thread_id is a UUID in session but not in map
// 3. New thread: neither path matches
use crate::agent::session::Thread;
let manager = SessionManager::new();
// Case 1: Normal resolution creates thread and maps it
let (session1, tid1) = manager
.resolve_thread("user1", "chan1", Some("ext-1"))
.await;
// Resolving again with same key should return same thread (fast path)
let (_, tid1_again) = manager
.resolve_thread("user1", "chan1", Some("ext-1"))
.await;
assert_eq!(tid1, tid1_again);
// Case 2: UUID adoption - insert a thread directly into session
let adopted_id = Uuid::new_v4();
{
let mut sess = session1.lock().await;
let thread = Thread::with_id(adopted_id, sess.id);
sess.threads.insert(adopted_id, thread);
}
// Resolve with the UUID as external_thread_id -- should adopt it
let (_, resolved) = manager
.resolve_thread("user1", "chan1", Some(&adopted_id.to_string()))
.await;
assert_eq!(resolved, adopted_id);
// Case 3: Different channel gets different thread
let (_, tid2) = manager.resolve_thread("user1", "chan2", None).await;
assert_ne!(tid1, tid2);
}
#[tokio::test]
async fn test_resolve_thread_finds_existing_session_thread_by_uuid() {
use crate::agent::session::{Session, Thread};
+163 -23
View File
@@ -25,6 +25,25 @@ use crate::tools::redact_params;
const FORGED_THREAD_ID_ERROR: &str = "Invalid or unauthorized thread ID.";
/// Result of attempting to hydrate a thread from the database.
///
/// Distinguishes between a thread that was fully hydrated into the session
/// (messages loaded, thread registered) and one where we recognised the UUID
/// but skipped hydration (e.g. already present in memory, or ownership could
/// not be verified on a non-gateway channel).
#[derive(Debug)]
#[allow(dead_code)] // Inner UUIDs are part of the API contract for future callers
pub(super) enum HydrationResult {
/// Thread hydrated and available in `sess.threads` / `thread_map`.
Ready(Uuid),
/// UUID is known but hydration was intentionally skipped. The thread may
/// already be in memory, or the caller is on a channel that does not
/// require pre-existing threads so we fall through to `resolve_thread`.
Skipped(Uuid),
/// The external thread ID was not a valid UUID — nothing to hydrate.
NotFound,
}
fn requires_preexisting_uuid_thread(channel: &str) -> bool {
// Gateway-style channels send server-issued conversation UUIDs.
// Unknown UUIDs should be rejected instead of silently creating a new thread.
@@ -41,15 +60,24 @@ impl Agent {
/// even when the conversation has zero messages (e.g. a brand-new
/// assistant thread). Without this, `resolve_thread` would mint a
/// fresh UUID and all messages would land in the wrong conversation.
///
/// Returns [`HydrationResult::Ready`] when the thread was fully loaded
/// into the session, [`HydrationResult::Skipped`] when the UUID was
/// recognised but hydration was not performed (already in memory, or
/// ownership unverifiable on a non-gateway channel), and
/// [`HydrationResult::NotFound`] when the external ID is not a UUID.
///
/// Returns `Err` only for hard rejections (forged / unauthorised thread
/// ID on a gateway channel).
pub(super) async fn maybe_hydrate_thread(
&self,
message: &IncomingMessage,
external_thread_id: &str,
) -> Option<String> {
) -> Result<HydrationResult, String> {
// Only hydrate UUID-shaped thread IDs (web gateway uses UUIDs)
let thread_uuid = match Uuid::parse_str(external_thread_id) {
Ok(id) => id,
Err(_) => return None,
Err(_) => return Ok(HydrationResult::NotFound),
};
// Check if already in memory
@@ -60,7 +88,7 @@ impl Agent {
{
let sess = session.lock().await;
if sess.threads.contains_key(&thread_uuid) {
return None;
return Ok(HydrationResult::Skipped(thread_uuid));
}
}
@@ -83,9 +111,9 @@ impl Agent {
e
);
if requires_preexisting_uuid_thread(&message.channel) {
return Some(FORGED_THREAD_ID_ERROR.to_string());
return Err(FORGED_THREAD_ID_ERROR.to_string());
}
return None;
return Ok(HydrationResult::Skipped(thread_uuid));
}
};
if !owned {
@@ -99,9 +127,9 @@ impl Agent {
e
);
if requires_preexisting_uuid_thread(&message.channel) {
return Some(FORGED_THREAD_ID_ERROR.to_string());
return Err(FORGED_THREAD_ID_ERROR.to_string());
}
return None;
return Ok(HydrationResult::Skipped(thread_uuid));
}
};
@@ -113,7 +141,7 @@ impl Agent {
exists,
"Rejected message for unavailable thread id"
);
return Some(FORGED_THREAD_ID_ERROR.to_string());
return Err(FORGED_THREAD_ID_ERROR.to_string());
}
tracing::warn!(
@@ -122,7 +150,7 @@ impl Agent {
exists,
"Skipped hydration for thread id not owned by sender"
);
return None;
return Ok(HydrationResult::Skipped(thread_uuid));
}
let db_messages = store
@@ -169,7 +197,7 @@ impl Agent {
msg_count
);
None
Ok(HydrationResult::Ready(thread_uuid))
}
pub(super) async fn process_user_input(
@@ -1413,8 +1441,20 @@ impl Agent {
{
let mut sess = session.lock().await;
if let Some(thread) = sess.threads.get_mut(&thread_id) {
thread.await_approval(new_pending);
match sess.threads.get_mut(&thread_id) {
Some(thread) => {
thread.await_approval(new_pending);
}
None => {
tracing::error!(
%thread_id,
tool = %tool_name,
"Thread disappeared while preparing approval request"
);
return Ok(SubmissionResult::error(
"The conversation thread was pruned during processing. Some actions may have already been executed. Please check results before retrying.",
));
}
}
}
@@ -1546,17 +1586,25 @@ impl Agent {
);
{
let mut sess = session.lock().await;
if let Some(thread) = sess.threads.get_mut(&thread_id) {
thread.clear_pending_approval();
thread.complete_turn(&rejection);
// User message already persisted at turn start; save rejection response
self.persist_assistant_response(
thread_id,
&message.channel,
&message.user_id,
&rejection,
)
.await;
match sess.threads.get_mut(&thread_id) {
Some(thread) => {
thread.clear_pending_approval();
thread.complete_turn(&rejection);
// User message already persisted at turn start; save rejection response
self.persist_assistant_response(
thread_id,
&message.channel,
&message.user_id,
&rejection,
)
.await;
}
None => {
tracing::warn!(
%thread_id,
"Thread disappeared during approval rejection — rejection not persisted"
);
}
}
}
@@ -2204,6 +2252,98 @@ mod tests {
assert!(t.pending_messages.is_empty());
}
/// Regression test for #1487: when a thread disappears from the session during
/// approval storage, the code should return an error instead of silently losing
/// the approval.
#[test]
fn test_missing_thread_during_approval_storage_returns_error() {
use crate::agent::session::{PendingApproval, Session};
use uuid::Uuid;
let thread_id = Uuid::new_v4();
let session = Session::new("test-user");
// Thread does NOT exist in the session
assert!(!session.threads.contains_key(&thread_id));
// Simulate the match logic from process_approval when storing a new pending approval
let _new_pending = PendingApproval {
request_id: Uuid::new_v4(),
tool_name: "shell".to_string(),
parameters: serde_json::json!({"command": "echo test"}),
display_parameters: serde_json::json!({"command": "[REDACTED]"}),
description: "Execute command".to_string(),
tool_call_id: "call_0".to_string(),
context_messages: vec![],
deferred_tool_calls: vec![],
user_timezone: None,
allow_always: false,
};
let tool_name = "shell";
// The fixed code uses match instead of if-let, returning an error for None
let result: Result<&str, String> = match session.threads.get(&thread_id) {
Some(_thread) => {
// Would call thread.await_approval(new_pending)
Ok("stored")
}
None => Err(format!(
"The conversation thread was pruned during processing. Some actions may have already been executed. Tool: {}",
tool_name,
)),
};
assert!(result.is_err(), "Missing thread should produce an error");
let err = result.unwrap_err();
assert!(
err.contains("pruned during processing"),
"Error should mention thread was pruned. Got: {}",
err
);
}
/// Regression test for #1487: when a thread disappears during rejection,
/// the rejection is not persisted but the code degrades gracefully (no panic,
/// no silent success pretending state was updated).
#[test]
fn test_missing_thread_during_rejection_degrades_gracefully() {
use crate::agent::session::Session;
use uuid::Uuid;
let thread_id = Uuid::new_v4();
let mut session = Session::new("test-user");
// Thread does NOT exist in the session
assert!(!session.threads.contains_key(&thread_id));
let rejection = format!(
"Tool '{}' was rejected. The agent will not execute this tool.",
"shell"
);
// The fixed code uses match instead of if-let, logging a warning for None
let mut persisted = false;
match session.threads.get_mut(&thread_id) {
Some(thread) => {
thread.clear_pending_approval();
thread.complete_turn(&rejection);
persisted = true;
}
None => {
// In production this logs a warning -- we just verify it takes
// the None branch without panicking.
}
}
assert!(
!persisted,
"Rejection should NOT be persisted when thread is missing"
);
// Session should remain unchanged
assert!(session.threads.is_empty());
}
// Helper function to extract the approval message without needing a full Agent instance
fn extract_approval_message(
session: &crate::agent::session::Session,
-11
View File
@@ -325,20 +325,9 @@ 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));
+1 -7
View File
@@ -1822,13 +1822,7 @@ async fn memory_write_handler(
"Workspace not available".to_string(),
))?;
// 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.
// Route through layer-aware methods when a layer is specified
if let Some(ref layer_name) = req.layer {
let result = if req.append {
workspace
-10
View File
@@ -25,7 +25,6 @@ pub mod import;
mod logs;
mod mcp;
pub mod memory;
mod models;
pub mod oauth_defaults;
mod pairing;
mod registry;
@@ -46,7 +45,6 @@ pub use logs::{LogsCommand, run_logs_command};
pub use mcp::{McpCommand, run_mcp_command};
pub use memory::MemoryCommand;
pub use memory::run_memory_command_with_db;
pub use models::{ModelsCommand, run_models_command};
pub use pairing::{PairingCommand, run_pairing_command, run_pairing_command_with_store};
pub use registry::{RegistryCommand, run_registry_command};
pub use routines::{RoutinesCommand, run_routines_command};
@@ -219,14 +217,6 @@ pub enum Command {
)]
Hooks(HooksCommand),
/// Manage LLM providers and models
#[command(
subcommand,
about = "Manage LLM providers and models",
long_about = "List providers, view current configuration, and set active provider/model.\nExamples:\n ironclaw models list\n ironclaw models list openai --verbose\n ironclaw models status\n ironclaw models set gpt-4o\n ironclaw models set-provider anthropic --model claude-sonnet-4-6-20250514"
)]
Models(ModelsCommand),
/// Probe external dependencies and validate configuration
#[command(
about = "Run diagnostics",
-864
View File
@@ -1,864 +0,0 @@
//! Models management CLI commands.
//!
//! Provides subcommands for listing providers, viewing current model
//! configuration, and setting the active provider/model. Settings are
//! persisted to both `config.toml` and `~/.ironclaw/.env` so changes
//! take effect immediately (no DB connection required).
use clap::Subcommand;
use std::path::Path;
use crate::llm::registry::ProviderRegistry;
use crate::settings::Settings;
#[derive(Subcommand, Debug, Clone)]
pub enum ModelsCommand {
/// List providers (or available models for a specific provider)
List {
/// Show only a specific provider (by ID or alias)
provider: Option<String>,
/// Show detailed information (env vars, base URL, protocol)
#[arg(short, long)]
verbose: bool,
/// Output as JSON
#[arg(long)]
json: bool,
},
/// Show current model configuration
Status {
/// Output as JSON
#[arg(long)]
json: bool,
},
/// Set the default model
Set {
/// Model name (e.g., "gpt-5-mini", "claude-sonnet-4-6-20250514")
model: String,
},
/// Set the LLM provider
SetProvider {
/// Provider ID or alias (e.g., "openai", "anthropic", "ollama")
provider: String,
/// Also set the model (defaults to provider's default model)
#[arg(long)]
model: Option<String>,
},
}
/// Run the models CLI subcommand.
pub async fn run_models_command(
cmd: ModelsCommand,
config_path: Option<&Path>,
) -> anyhow::Result<()> {
match cmd {
ModelsCommand::List {
provider,
verbose,
json,
} => {
if let Some(ref id) = provider {
cmd_show_provider(id, verbose, json, config_path).await
} else {
cmd_list_providers(verbose, json, config_path).await
}
}
ModelsCommand::Status { json } => cmd_status(json, config_path),
ModelsCommand::Set { model } => cmd_set_model(&model, config_path),
ModelsCommand::SetProvider { provider, model } => {
cmd_set_provider(&provider, model.as_deref(), config_path)
}
}
}
// ─── Shared helpers ───────────────────────────────────────────────
/// Resolve the currently active backend and model from env + settings.
fn resolve_active(config_path: Option<&Path>) -> (String, String) {
let settings = load_settings(config_path);
resolve_active_from_settings(&settings)
}
/// Resolve active backend + model from a pre-loaded Settings.
fn resolve_active_from_settings(settings: &Settings) -> (String, String) {
let backend = std::env::var("LLM_BACKEND")
.ok()
.or_else(|| settings.llm_backend.clone())
.unwrap_or_else(|| "nearai".to_string());
let registry = ProviderRegistry::load();
let canonical_backend = registry
.find(&backend)
.map(|d| d.id.clone())
.unwrap_or_else(|| backend.clone());
let model = if canonical_backend == "nearai" {
std::env::var("NEARAI_MODEL")
.ok()
.or_else(|| settings.selected_model.clone())
.unwrap_or_else(|| "qwen2.5-72b-instruct:free".to_string())
} else if let Some(def) = registry.find(&canonical_backend) {
std::env::var(&def.model_env)
.ok()
.or_else(|| settings.selected_model.clone())
.unwrap_or_else(|| def.default_model.clone())
} else {
settings
.selected_model
.clone()
.unwrap_or_else(|| "unknown".to_string())
};
(canonical_backend, model)
}
fn load_settings(config_path: Option<&Path>) -> Settings {
if let Some(path) = config_path {
Settings::load_toml(path).ok().flatten().unwrap_or_default()
} else {
let toml_path = config_toml_path();
if toml_path.exists() {
Settings::load_toml(&toml_path)
.ok()
.flatten()
.unwrap_or_default()
} else {
Settings::load()
}
}
}
fn save_settings(settings: &Settings, config_path: Option<&Path>) -> anyhow::Result<()> {
let path = config_path
.map(|p| p.to_path_buf())
.unwrap_or_else(config_toml_path);
settings
.save_toml(&path)
.map_err(|e| anyhow::anyhow!("{}", e))?;
Ok(())
}
fn config_toml_path() -> std::path::PathBuf {
crate::bootstrap::ironclaw_base_dir().join("config.toml")
}
/// Try to fetch the live model list from a provider.
///
/// Best-effort: returns `None` if config loading, provider creation, or the
/// `list_models()` call fails (missing API key, network error, etc.).
async fn try_fetch_models(provider_id: &str, config_path: Option<&Path>) -> Option<Vec<String>> {
let config = crate::config::Config::from_env_with_toml(config_path)
.await
.ok()?;
// Override backend to the requested provider so create_llm_provider
// constructs the right one.
let mut llm_config = config.llm.clone();
llm_config.backend = provider_id.to_string();
// For registry providers, resolve the RegistryProviderConfig if not
// already set for this backend.
if provider_id != "nearai" && provider_id != "bedrock" {
let registry = ProviderRegistry::load();
if let Some(def) = registry.find(provider_id)
&& llm_config
.provider
.as_ref()
.is_none_or(|p| p.provider_id != def.id)
{
// Build a minimal RegistryProviderConfig from env + registry
let api_key = def
.api_key_env
.as_ref()
.and_then(|env| std::env::var(env).ok());
if def.api_key_required && api_key.is_none() {
return None;
}
let base_url = def.default_base_url.clone().unwrap_or_default();
llm_config.provider = Some(crate::llm::RegistryProviderConfig {
protocol: def.protocol,
provider_id: def.id.clone(),
model: def.default_model.clone(),
api_key: api_key.map(secrecy::SecretString::from),
base_url,
extra_headers: Vec::new(),
oauth_token: None,
is_codex_chatgpt: false,
refresh_token: None,
auth_path: None,
cache_retention: Default::default(),
unsupported_params: def.unsupported_params.clone(),
});
}
}
let session = crate::llm::create_session_manager(config.llm.session.clone()).await;
let provider = crate::llm::create_llm_provider(&llm_config, session)
.await
.ok()?;
provider.list_models().await.ok().filter(|m| !m.is_empty())
}
/// Print available models section (text output).
fn print_model_list(models: &Option<Vec<String>>, active_model: Option<&String>) {
match models {
Some(models) => {
println!("\n Available models ({}):", models.len());
for m in models {
let marker = active_model
.filter(|a| a.as_str() == m)
.map(|_| " (active)")
.unwrap_or("");
println!(" {}{}", m, marker);
}
}
None => {
println!(
"\n Could not fetch model list (missing credentials or provider unavailable)."
);
}
}
}
/// Also update `~/.ironclaw/.env` so changes take effect immediately.
///
/// Skipped when `config_path` is `Some` (custom `--config`), because the user
/// is explicitly targeting a different config file and we must not pollute the
/// default profile's `.env`.
fn sync_to_dotenv(config_path: Option<&Path>, vars: &[(&str, &str)]) {
if config_path.is_some() {
return;
}
if let Err(e) = crate::bootstrap::upsert_bootstrap_vars(vars) {
eprintln!("Warning: failed to update .env: {}", e);
}
}
// ─── status ───────────────────────────────────────────────────────
fn cmd_status(json: bool, config_path: Option<&Path>) -> anyhow::Result<()> {
let settings = load_settings(config_path);
let (backend, model) = resolve_active_from_settings(&settings);
let registry = ProviderRegistry::load();
let fallback = std::env::var("NEARAI_FALLBACK_MODEL").ok();
let cheap = std::env::var("NEARAI_CHEAP_MODEL").ok();
let description = if backend == "nearai" {
"NEAR AI inference (default)".to_string()
} else {
registry
.find(&backend)
.map(|d| d.description.clone())
.unwrap_or_default()
};
if json {
let v = serde_json::json!({
"provider": backend,
"model": model,
"description": description,
"fallback_model": fallback,
"cheap_model": cheap,
});
println!(
"{}",
serde_json::to_string_pretty(&v).unwrap_or_else(|_| "{}".to_string())
);
return Ok(());
}
println!("Provider: {} ({})", backend, description);
println!("Model: {}", model);
if let Some(ref fb) = fallback {
println!("Fallback: {}", fb);
}
if let Some(ref ch) = cheap {
println!("Cheap: {}", ch);
}
Ok(())
}
// ─── set ──────────────────────────────────────────────────────────
fn cmd_set_model(model: &str, config_path: Option<&Path>) -> anyhow::Result<()> {
let trimmed = model.trim();
if trimmed.is_empty() {
anyhow::bail!("Model name cannot be empty");
}
let mut settings = load_settings(config_path);
let registry = ProviderRegistry::load();
// Warn if model name doesn't match any known provider's default model
let known_model = registry.all().iter().any(|d| d.default_model == trimmed)
|| trimmed.contains("qwen") // nearai models
|| trimmed.contains("llama")
|| trimmed.contains("gpt")
|| trimmed.contains("claude")
|| trimmed.contains("gemini")
|| trimmed.contains("mistral");
if !known_model {
eprintln!(
"Warning: '{}' is not a recognized model name. Proceeding anyway.",
trimmed
);
}
settings.selected_model = Some(trimmed.to_string());
save_settings(&settings, config_path)?;
let backend = std::env::var("LLM_BACKEND")
.ok()
.or_else(|| settings.llm_backend.clone())
.unwrap_or_else(|| "nearai".to_string());
// Also write to .env so the change takes effect immediately
let model_env = if backend == "nearai" {
"NEARAI_MODEL".to_string()
} else {
registry
.find(&backend)
.map(|d| d.model_env.clone())
.unwrap_or_default()
};
if !model_env.is_empty() {
sync_to_dotenv(config_path, &[(&model_env, trimmed)]);
}
println!("Model set to '{}' (provider: {})", trimmed, backend);
println!(
"Saved to {}",
config_path
.map(|p| p.display().to_string())
.unwrap_or_else(|| config_toml_path().display().to_string())
);
Ok(())
}
// ─── set-provider ─────────────────────────────────────────────────
fn cmd_set_provider(
provider: &str,
model: Option<&str>,
config_path: Option<&Path>,
) -> anyhow::Result<()> {
let registry = ProviderRegistry::load();
// Validate and normalize provider
let canonical_id = if provider == "nearai" || provider == "near_ai" || provider == "near" {
"nearai".to_string()
} else {
let def = registry.find(provider).ok_or_else(|| {
let known: Vec<&str> = std::iter::once("nearai")
.chain(registry.all().iter().map(|d| d.id.as_str()))
.collect();
anyhow::anyhow!(
"Unknown provider '{}'. Known providers: {}",
provider,
known.join(", ")
)
})?;
def.id.clone()
};
// Resolve model: explicit > provider default
let resolved_model = if let Some(m) = model {
m.to_string()
} else if canonical_id == "nearai" {
"qwen2.5-72b-instruct:free".to_string()
} else if let Some(def) = registry.find(&canonical_id) {
def.default_model.clone()
} else {
"default".to_string()
};
let mut settings = load_settings(config_path);
settings.llm_backend = Some(canonical_id.clone());
settings.selected_model = Some(resolved_model.clone());
save_settings(&settings, config_path)?;
// Also write to .env so the change takes effect immediately
let model_env = if canonical_id == "nearai" {
"NEARAI_MODEL".to_string()
} else {
registry
.find(&canonical_id)
.map(|d| d.model_env.clone())
.unwrap_or_default()
};
let mut vars: Vec<(&str, &str)> = vec![("LLM_BACKEND", &canonical_id)];
if !model_env.is_empty() {
vars.push((&model_env, &resolved_model));
}
sync_to_dotenv(config_path, &vars);
println!(
"Provider set to '{}', model set to '{}'",
canonical_id, resolved_model
);
println!(
"Saved to {}",
config_path
.map(|p| p.display().to_string())
.unwrap_or_else(|| config_toml_path().display().to_string())
);
Ok(())
}
// ─── list ─────────────────────────────────────────────────────────
/// List all providers with their default models.
async fn cmd_list_providers(
verbose: bool,
json: bool,
config_path: Option<&Path>,
) -> anyhow::Result<()> {
let registry = ProviderRegistry::load();
let (active_backend, active_model) = resolve_active(config_path);
if json {
let mut entries: Vec<serde_json::Value> = Vec::new();
// NEAR AI (not in registry)
let nearai_active = active_backend == "nearai";
entries.push(serde_json::json!({
"id": "nearai",
"description": "NEAR AI inference (default)",
"default_model": "qwen2.5-72b-instruct:free",
"active": nearai_active,
"active_model": if nearai_active { Some(&active_model) } else { None },
}));
for def in registry.all() {
let is_active = active_backend == def.id;
let mut v = serde_json::json!({
"id": def.id,
"description": def.description,
"default_model": def.default_model,
"protocol": format!("{:?}", def.protocol),
"active": is_active,
});
if is_active {
v["active_model"] = serde_json::json!(active_model);
}
if verbose {
v["aliases"] = serde_json::json!(def.aliases);
v["model_env"] = serde_json::json!(def.model_env);
v["api_key_env"] = serde_json::json!(def.api_key_env);
v["api_key_required"] = serde_json::json!(def.api_key_required);
if let Some(ref url) = def.default_base_url {
v["base_url"] = serde_json::json!(url);
}
if let Some(ref setup) = def.setup {
v["can_list_models"] = serde_json::json!(setup.can_list_models());
}
}
entries.push(v);
}
println!(
"{}",
serde_json::to_string_pretty(&entries).unwrap_or_else(|_| "[]".to_string())
);
return Ok(());
}
let providers = registry.all();
println!("Active: {} (model: {})\n", active_backend, active_model);
println!(
"{} provider(s) available:\n",
providers.len() + 1 // +1 for NEAR AI
);
// NEAR AI (not in registry)
let nearai_marker = if active_backend == "nearai" { " *" } else { "" };
if verbose {
println!(" nearai{}", nearai_marker);
println!(" Description: NEAR AI inference (default)");
println!(" Default model: qwen2.5-72b-instruct:free");
println!(" Model env: NEARAI_MODEL");
if active_backend == "nearai" {
println!(" Active model: {}", active_model);
}
println!();
} else {
println!(
" {:<22} {:<40} NEAR AI inference (default)",
format!("nearai{nearai_marker}"),
"qwen2.5-72b-instruct:free"
);
}
for def in providers {
let is_active = active_backend == def.id;
let marker = if is_active { " *" } else { "" };
if verbose {
println!(" {}{}", def.id, marker);
println!(" Description: {}", def.description);
println!(" Default model: {}", def.default_model);
println!(" Protocol: {:?}", def.protocol);
println!(" Model env: {}", def.model_env);
if let Some(ref env) = def.api_key_env {
println!(
" API key env: {} ({})",
env,
if def.api_key_required {
"required"
} else {
"optional"
}
);
}
if let Some(ref url) = def.default_base_url {
println!(" Base URL: {}", url);
}
if !def.aliases.is_empty() {
println!(" Aliases: {}", def.aliases.join(", "));
}
if is_active {
println!(" Active model: {}", active_model);
}
println!();
} else {
let model_display = if is_active {
active_model.clone()
} else {
def.default_model.clone()
};
println!(
" {:<22} {:<40} {}",
format!("{}{marker}", def.id),
model_display,
def.description,
);
}
}
if !verbose {
println!();
println!("* = active provider. Use --verbose for details.");
}
Ok(())
}
/// Show details for a specific provider.
async fn cmd_show_provider(
id: &str,
verbose: bool,
json: bool,
config_path: Option<&Path>,
) -> anyhow::Result<()> {
let registry = ProviderRegistry::load();
let (active_backend, active_model) = resolve_active(config_path);
// Resolve canonical ID for model fetching
let canonical_id = if id == "nearai" || id == "near_ai" || id == "near" {
"nearai".to_string()
} else {
registry
.find(id)
.map(|d| d.id.clone())
.unwrap_or_else(|| id.to_string())
};
// Try to fetch live model list from the provider
let live_models = try_fetch_models(&canonical_id, config_path).await;
// Check NEAR AI first (not in registry)
if id == "nearai" || id == "near_ai" || id == "near" {
let is_active = active_backend == "nearai";
if json {
let mut v = serde_json::json!({
"id": "nearai",
"description": "NEAR AI inference (default)",
"default_model": "qwen2.5-72b-instruct:free",
"model_env": "NEARAI_MODEL",
"active": is_active,
});
if is_active {
v["active_model"] = serde_json::json!(active_model);
}
if let Some(ref models) = live_models {
v["available_models"] = serde_json::json!(models);
}
println!(
"{}",
serde_json::to_string_pretty(&v).unwrap_or_else(|_| "{}".to_string())
);
} else {
println!("Provider: nearai");
println!(" Description: NEAR AI inference (default)");
println!(" Default model: qwen2.5-72b-instruct:free");
println!(" Model env: NEARAI_MODEL");
println!(" Active: {}", if is_active { "yes" } else { "no" });
if is_active {
println!(" Active model: {}", active_model);
}
print_model_list(&live_models, is_active.then_some(&active_model));
}
return Ok(());
}
let def = registry.find(id).ok_or_else(|| {
let known: Vec<&str> = std::iter::once("nearai")
.chain(registry.all().iter().map(|d| d.id.as_str()))
.collect();
anyhow::anyhow!(
"Unknown provider '{}'. Known providers: {}",
id,
known.join(", ")
)
})?;
let is_active = active_backend == def.id;
if json {
let mut v = serde_json::json!({
"id": def.id,
"description": def.description,
"protocol": format!("{:?}", def.protocol),
"default_model": def.default_model,
"model_env": def.model_env,
"api_key_env": def.api_key_env,
"api_key_required": def.api_key_required,
"aliases": def.aliases,
"active": is_active,
});
if let Some(ref url) = def.default_base_url {
v["base_url"] = serde_json::json!(url);
}
if let Some(ref setup) = def.setup {
v["can_list_models"] = serde_json::json!(setup.can_list_models());
v["display_name"] = serde_json::json!(setup.display_name());
}
if is_active {
v["active_model"] = serde_json::json!(active_model);
}
if verbose && !def.unsupported_params.is_empty() {
v["unsupported_params"] = serde_json::json!(def.unsupported_params);
}
if let Some(ref models) = live_models {
v["available_models"] = serde_json::json!(models);
}
println!(
"{}",
serde_json::to_string_pretty(&v).unwrap_or_else(|_| "{}".to_string())
);
return Ok(());
}
println!("Provider: {}", def.id);
println!(" Description: {}", def.description);
println!(" Protocol: {:?}", def.protocol);
println!(" Default model: {}", def.default_model);
println!(" Model env: {}", def.model_env);
if let Some(ref env) = def.api_key_env {
println!(
" API key env: {} ({})",
env,
if def.api_key_required {
"required"
} else {
"optional"
}
);
}
if let Some(ref url) = def.default_base_url {
println!(" Base URL: {}", url);
}
if !def.aliases.is_empty() {
println!(" Aliases: {}", def.aliases.join(", "));
}
if let Some(ref setup) = def.setup {
println!(
" List models: {}",
if setup.can_list_models() {
"supported"
} else {
"not supported"
}
);
println!(" Display name: {}", setup.display_name());
}
if !def.unsupported_params.is_empty() {
println!(" Unsupported: {}", def.unsupported_params.join(", "));
}
println!(" Active: {}", if is_active { "yes" } else { "no" });
if is_active {
println!(" Active model: {}", active_model);
}
print_model_list(&live_models, is_active.then_some(&active_model));
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn resolve_active_defaults_to_nearai() {
let settings = Settings::default();
assert!(settings.llm_backend.is_none());
assert!(settings.selected_model.is_none());
}
#[test]
fn registry_loads_all_providers() {
let registry = ProviderRegistry::load();
let all = registry.all();
assert!(
all.len() >= 10,
"should have at least 10 built-in providers, got {}",
all.len()
);
}
#[test]
fn registry_find_by_alias() {
let registry = ProviderRegistry::load();
let def = registry
.find("claude")
.expect("claude alias should resolve");
assert_eq!(def.id, "anthropic");
}
#[test]
fn all_providers_have_description() {
let registry = ProviderRegistry::load();
for def in registry.all() {
assert!(
!def.description.is_empty(),
"provider {} should have a description",
def.id
);
}
}
#[test]
fn set_model_persists_to_toml() {
let dir = tempfile::tempdir().expect("create temp dir");
let toml_path = dir.path().join("config.toml");
cmd_set_model("gpt-5-mini", Some(&toml_path)).expect("set model");
let settings = Settings::load_toml(&toml_path)
.expect("read toml")
.expect("should have settings");
assert_eq!(settings.selected_model.as_deref(), Some("gpt-5-mini"));
}
#[test]
fn set_provider_validates_unknown() {
let dir = tempfile::tempdir().expect("create temp dir");
let toml_path = dir.path().join("config.toml");
let result = cmd_set_provider("nonexistent_provider", None, Some(&toml_path));
assert!(result.is_err());
let err = result.unwrap_err().to_string();
assert!(
err.contains("Unknown provider"),
"should mention unknown provider: {}",
err
);
}
#[test]
fn set_provider_persists_to_toml() {
let dir = tempfile::tempdir().expect("create temp dir");
let toml_path = dir.path().join("config.toml");
cmd_set_provider("groq", None, Some(&toml_path)).expect("set provider");
let settings = Settings::load_toml(&toml_path)
.expect("read toml")
.expect("should have settings");
assert_eq!(settings.llm_backend.as_deref(), Some("groq"));
assert_eq!(
settings.selected_model.as_deref(),
Some("llama-3.3-70b-versatile")
);
}
#[test]
fn set_provider_with_custom_model() {
let dir = tempfile::tempdir().expect("create temp dir");
let toml_path = dir.path().join("config.toml");
cmd_set_provider("anthropic", Some("claude-opus-4-6"), Some(&toml_path))
.expect("set provider with model");
let settings = Settings::load_toml(&toml_path)
.expect("read toml")
.expect("should have settings");
assert_eq!(settings.llm_backend.as_deref(), Some("anthropic"));
assert_eq!(settings.selected_model.as_deref(), Some("claude-opus-4-6"));
}
#[test]
fn custom_config_does_not_pollute_default_dotenv() {
let dir = tempfile::tempdir().expect("create temp dir");
let toml_path = dir.path().join("config.toml");
// With a custom config path, sync_to_dotenv should be a no-op
// (it returns early when config_path is Some).
// We verify by checking that cmd_set_provider succeeds without
// trying to write to the default ~/.ironclaw/.env.
cmd_set_provider("groq", None, Some(&toml_path)).expect("set provider with custom config");
let settings = Settings::load_toml(&toml_path)
.expect("read toml")
.expect("should have settings");
assert_eq!(settings.llm_backend.as_deref(), Some("groq"));
// The key assertion is that no error was thrown trying to write
// to the default .env — sync_to_dotenv skipped it.
}
#[test]
fn set_model_rejects_empty_name() {
let dir = tempfile::tempdir().expect("create temp dir");
let toml_path = dir.path().join("config.toml");
let result = cmd_set_model("", Some(&toml_path));
assert!(result.is_err());
assert!(
result.unwrap_err().to_string().contains("cannot be empty"),
"should reject empty model name"
);
let result2 = cmd_set_model(" ", Some(&toml_path));
assert!(result2.is_err());
}
#[test]
fn set_provider_normalizes_alias() {
let dir = tempfile::tempdir().expect("create temp dir");
let toml_path = dir.path().join("config.toml");
cmd_set_provider("claude", None, Some(&toml_path)).expect("set via alias");
let settings = Settings::load_toml(&toml_path)
.expect("read toml")
.expect("should have settings");
assert_eq!(
settings.llm_backend.as_deref(),
Some("anthropic"),
"alias should be normalized to canonical ID"
);
}
}
@@ -20,7 +20,6 @@ Commands:
service Manage OS service
skills Manage skills
hooks Manage lifecycle hooks
models Manage LLM providers and models
doctor Run diagnostics
logs View and manage gateway logs
status Show system status
@@ -20,7 +20,6 @@ Commands:
service Manage OS service
skills Manage skills
hooks Manage lifecycle hooks
models Manage LLM providers and models
doctor Run diagnostics
logs View and manage gateway logs
status Show system status
@@ -23,7 +23,6 @@ Commands:
service Manage OS service
skills Manage skills
hooks Manage lifecycle hooks
models Manage LLM providers and models
doctor Run diagnostics
logs View and manage gateway logs
status Show system status
@@ -23,7 +23,6 @@ Commands:
service Manage OS service
skills Manage skills
hooks Manage lifecycle hooks
models Manage LLM providers and models
doctor Run diagnostics
logs View and manage gateway logs
status Show system status
+7 -8
View File
@@ -24,7 +24,7 @@ mod skills;
mod transcription;
mod tunnel;
mod wasm;
pub(crate) mod workspace;
mod workspace;
use std::collections::HashMap;
use std::sync::{LazyLock, Mutex, Once};
@@ -178,7 +178,9 @@ impl Config {
},
transcription: TranscriptionConfig::default(),
search: WorkspaceSearchConfig::default(),
workspace: WorkspaceConfig::default(),
workspace: WorkspaceConfig {
memory_layers: vec![],
},
observability: crate::observability::ObservabilityConfig::default(),
relay: None,
}
@@ -311,14 +313,11 @@ 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.as_str())
.unwrap_or("default");
let workspace = WorkspaceConfig::resolve(workspace_user_id)?;
.map(|gw| gw.user_id.clone())
.unwrap_or_else(|| "default".to_string());
Ok(Self {
owner_id: owner_id.clone(),
@@ -340,7 +339,7 @@ impl Config {
skills: SkillsConfig::resolve()?,
transcription: TranscriptionConfig::resolve(settings)?,
search: WorkspaceSearchConfig::resolve()?,
workspace,
workspace: WorkspaceConfig::resolve(&workspace_user_id)?,
observability: crate::observability::ObservabilityConfig {
backend: std::env::var("OBSERVABILITY_BACKEND").unwrap_or_else(|_| "none".into()),
},
+7 -68
View File
@@ -2,29 +2,18 @@ use crate::config::helpers::optional_env;
use crate::error::ConfigError;
use crate::workspace::layer::MemoryLayer;
/// Workspace-level configuration (memory layers, read scopes).
/// Workspace memory configuration.
///
/// Parsed from environment variables. Lives outside of `GatewayConfig`
/// so that non-gateway channels can eventually use the same settings.
#[derive(Debug, Clone, Default)]
/// 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)]
pub struct WorkspaceConfig {
/// Memory layer definitions (JSON in `MEMORY_LAYERS` env var, or defaults).
pub memory_layers: Vec<MemoryLayer>,
/// 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<String>,
}
impl WorkspaceConfig {
/// 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<Self, ConfigError> {
// --- Memory layers ---
pub(crate) fn resolve(user_id: &str) -> Result<Self, ConfigError> {
let memory_layers: Vec<MemoryLayer> = match optional_env("MEMORY_LAYERS")? {
Some(json_str) => {
serde_json::from_str(&json_str).map_err(|e| ConfigError::InvalidValue {
@@ -68,20 +57,6 @@ 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
@@ -97,43 +72,7 @@ impl WorkspaceConfig {
}
}
// --- Read scopes ---
let read_scopes: Vec<String> = 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,
})
Ok(Self { memory_layers })
}
}
-97
View File
@@ -644,103 +644,6 @@ pub trait WorkspaceStore: Send + Sync {
embedding: Option<&[f32]>,
config: &SearchConfig,
) -> Result<Vec<SearchResult>, 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<Uuid>,
query: &str,
embedding: Option<&[f32]>,
config: &SearchConfig,
) -> Result<Vec<SearchResult>, 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<Uuid>,
) -> Result<Vec<String>, 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<Uuid>,
path: &str,
) -> Result<MemoryDocument, WorkspaceError> {
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<Uuid>,
directory: &str,
) -> Result<Vec<WorkspaceEntry>, 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.
-45
View File
@@ -717,49 +717,4 @@ 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<Uuid>,
query: &str,
embedding: Option<&[f32]>,
config: &SearchConfig,
) -> Result<Vec<SearchResult>, 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<Uuid>,
) -> Result<Vec<String>, 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<Uuid>,
path: &str,
) -> Result<MemoryDocument, WorkspaceError> {
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<Uuid>,
directory: &str,
) -> Result<Vec<WorkspaceEntry>, WorkspaceError> {
self.repo
.list_directory_multi(user_ids, agent_id, directory)
.await
}
}
+3
View File
@@ -304,6 +304,9 @@ 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 },
-5
View File
@@ -142,11 +142,6 @@ async fn async_main() -> anyhow::Result<()> {
init_cli_tracing();
return ironclaw::cli::run_logs_command(logs_cmd.clone(), cli.config.as_deref()).await;
}
Some(Command::Models(models_cmd)) => {
init_cli_tracing();
return ironclaw::cli::run_models_command(models_cmd.clone(), cli.config.as_deref())
.await;
}
Some(Command::Doctor) => {
init_cli_tracing();
return ironclaw::cli::run_doctor_command().await;
+1 -70
View File
@@ -28,7 +28,7 @@ use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
use async_trait::async_trait;
use rust_decimal::Decimal;
use tokio::sync::{Mutex as AsyncMutex, mpsc};
use tokio::sync::mpsc;
use crate::agent::AgentDeps;
use crate::channels::{
@@ -361,75 +361,6 @@ impl Channel for StubChannel {
}
}
/// Captured broadcast deliveries keyed by the target user or chat identifier.
pub type BroadcastCapture = Arc<AsyncMutex<Vec<(String, OutgoingResponse)>>>;
/// 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<MessageStream, ChannelError> {
let (_tx, rx) = mpsc::channel::<IncomingMessage>(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.
+4 -3
View File
@@ -271,13 +271,12 @@ 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"))
}
@@ -319,6 +318,8 @@ 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
+91 -76
View File
@@ -80,12 +80,6 @@ fn metadata_notify_user(metadata: &serde_json::Value) -> Option<String> {
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<String> {
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,
@@ -97,13 +91,11 @@ fn channel_matches_source(resolved_channel: Option<&str>, source_channel: Option
async fn resolve_channel_fallback_target(
extension_manager: Option<&Arc<ExtensionManager>>,
channel: Option<&str>,
owner_scope_target: Option<&str>,
ctx_user_id: &str,
) -> Option<String> {
// 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 channel_name = channel?;
if let Some(extension_manager) = extension_manager
&& let Some(target) = extension_manager
.notification_target_for_channel(channel_name)
.await
@@ -111,19 +103,13 @@ async fn resolve_channel_fallback_target(
return Some(target);
}
// `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()))
Some(ctx_user_id.to_string())
}
struct MessageTargetResolution<'a> {
extension_manager: Option<&'a Arc<ExtensionManager>>,
explicit_target: Option<String>,
metadata_target: Option<String>,
owner_scope_target: Option<String>,
default_target: Option<String>,
channel: Option<&'a str>,
metadata_channel: Option<&'a str>,
@@ -147,7 +133,6 @@ async fn resolve_message_target(inputs: MessageTargetResolution<'_>) -> Option<S
return resolve_channel_fallback_target(
inputs.extension_manager,
inputs.channel,
inputs.owner_scope_target.as_deref(),
inputs.ctx_user_id,
)
.await;
@@ -160,12 +145,9 @@ async fn resolve_message_target(inputs: MessageTargetResolution<'_>) -> Option<S
}
if inputs.channel.is_some() {
// Shared per-turn conversation defaults are already scoped to the
// active interactive target, so owner scope metadata is irrelevant.
return resolve_channel_fallback_target(
inputs.extension_manager,
inputs.channel,
None,
inputs.ctx_user_id,
)
.await;
@@ -242,9 +224,8 @@ impl Tool for MessageTool {
.unwrap_or_else(|e| e.into_inner())
.clone();
let metadata_target = metadata_notify_user(&ctx.metadata);
let owner_scope_target = metadata_owner_id(&ctx.metadata);
let has_execution_routing_metadata =
metadata_channel.is_some() || metadata_target.is_some() || owner_scope_target.is_some();
metadata_channel.is_some() || metadata_target.is_some();
// Job metadata is authoritative for autonomous executions. The shared
// conversation defaults are only a legacy fallback when no execution-local
@@ -269,7 +250,6 @@ impl Tool for MessageTool {
extension_manager: self.extension_manager.as_ref(),
explicit_target,
metadata_target,
owner_scope_target,
default_target,
channel: channel.as_deref(),
metadata_channel: metadata_channel.as_deref(),
@@ -425,13 +405,83 @@ impl Tool for MessageTool {
#[cfg(test)]
mod tests {
use super::*;
use crate::testing::{BroadcastCapture, RecordingBroadcastChannel};
use async_trait::async_trait;
use tokio::sync::{Mutex, mpsc};
use crate::channels::{
Channel, IncomingMessage, MessageStream, OutgoingResponse, StatusUpdate,
};
use crate::error::ChannelError;
type BroadcastCapture = Arc<Mutex<Vec<(String, OutgoingResponse)>>>;
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<MessageStream, ChannelError> {
let (_tx, rx) = mpsc::channel::<IncomingMessage>(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(())
}
}
async fn message_tool_with_recording_channels()
-> (MessageTool, BroadcastCapture, BroadcastCapture) {
let channel_manager = ChannelManager::new();
let (gateway, gateway_captures) = RecordingBroadcastChannel::new("gateway");
let (telegram, telegram_captures) = RecordingBroadcastChannel::new("telegram");
let (gateway, gateway_captures) = RecordingChannel::new("gateway");
let (telegram, telegram_captures) = RecordingChannel::new("telegram");
channel_manager.add(Box::new(gateway)).await;
channel_manager.add(Box::new(telegram)).await;
@@ -820,63 +870,28 @@ mod tests {
}
#[tokio::test]
async fn message_tool_falls_back_to_owner_scope_when_channel_known() {
let (tool, gateway_captures, telegram_captures) =
message_tool_with_recording_channels().await;
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()));
let mut ctx =
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",
);
crate::context::JobContext::with_user("owner-scope", "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
.expect(
"message tool should fall back to ctx.user_id when owner scope metadata is absent",
);
.await;
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");
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
}
#[tokio::test]
-65
View File
@@ -1438,9 +1438,6 @@ impl From<TaskOutput> for Result<String, Error> {
#[cfg(test)]
mod tests {
use std::sync::Arc;
use crate::channels::ChannelManager;
use crate::llm::ToolSelection;
use super::*;
@@ -1451,8 +1448,6 @@ 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.
@@ -1544,20 +1539,6 @@ mod tests {
Worker::new(job_id, deps)
}
async fn make_worker_with_message_tool()
-> (Worker, Arc<MessageTool>, 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 {
@@ -2166,50 +2147,4 @@ 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");
}
}
-21
View File
@@ -91,27 +91,6 @@ 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):
+4 -167
View File
@@ -37,25 +37,6 @@ 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 {
@@ -120,7 +101,10 @@ impl MemoryDocument {
/// Check if this is a well-known identity document.
pub fn is_identity_document(&self) -> bool {
is_identity_path(&self.path)
matches!(
self.path.as_str(),
paths::IDENTITY | paths::SOUL | paths::AGENTS | paths::USER
)
}
}
@@ -144,42 +128,6 @@ 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<Item = WorkspaceEntry>,
) -> Vec<WorkspaceEntry> {
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<WorkspaceEntry> = 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 {
@@ -278,115 +226,4 @@ 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");
}
}
+38 -362
View File
@@ -52,10 +52,7 @@ mod repository;
mod search;
pub use chunker::{ChunkConfig, chunk_document};
pub use document::{
IDENTITY_PATHS, MemoryChunk, MemoryDocument, WorkspaceEntry, is_identity_path,
merge_workspace_entries, paths,
};
pub use document::{MemoryChunk, MemoryDocument, WorkspaceEntry, paths};
pub use embedding_cache::{CachedEmbeddingProvider, EmbeddingCacheConfig};
pub use embeddings::{
EmbeddingProvider, MockEmbeddings, NearAiEmbeddings, OllamaEmbeddings, OpenAiEmbeddings,
@@ -323,48 +320,6 @@ impl WorkspaceStorage {
}
}
}
// ==================== Multi-scope read methods ====================
async fn hybrid_search_multi(
&self,
user_ids: &[String],
agent_id: Option<Uuid>,
query: &str,
embedding: Option<&[f32]>,
config: &SearchConfig,
) -> Result<Vec<SearchResult>, 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<Uuid>,
path: &str,
) -> Result<MemoryDocument, WorkspaceError> {
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.
@@ -385,20 +340,9 @@ 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). All writes go to this scope.
/// User identifier (from channel).
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<String>,
/// Optional agent ID for multi-agent isolation.
agent_id: Option<Uuid>,
/// Database storage backend.
@@ -427,7 +371,6 @@ 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)),
@@ -447,7 +390,6 @@ 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),
@@ -532,12 +474,6 @@ impl Workspace {
///
/// Also updates read_user_ids to include all layer scopes.
pub fn with_memory_layers(mut self, layers: Vec<crate::workspace::layer::MemoryLayer>) -> 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
}
@@ -560,37 +496,11 @@ impl Workspace {
&self.memory_layers
}
/// 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<String>) -> 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).
/// Get the user ID.
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<Uuid> {
self.agent_id
@@ -608,33 +518,6 @@ impl Workspace {
/// println!("{}", doc.content);
/// ```
pub async fn read(&self, path: &str) -> Result<MemoryDocument, WorkspaceError> {
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<MemoryDocument, WorkspaceError> {
let path = normalize_path(path);
self.storage
.get_document_by_path(&self.user_id, self.agent_id, &path)
@@ -673,9 +556,6 @@ 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.
@@ -796,20 +676,6 @@ 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,
@@ -840,25 +706,13 @@ 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<bool, WorkspaceError> {
let path = normalize_path(path);
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 {
match self
.storage
.get_document_by_path(&self.user_id, self.agent_id, &path)
.await
{
Ok(_) => Ok(true),
Err(WorkspaceError::DocumentNotFound { .. }) => Ok(false),
Err(e) => Err(e),
@@ -893,55 +747,16 @@ impl Workspace {
/// ```
pub async fn list(&self, directory: &str) -> Result<Vec<WorkspaceEntry>, WorkspaceError> {
let directory = normalize_directory(directory);
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
}
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<Vec<String>, WorkspaceError> {
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
}
self.storage
.list_all_paths(&self.user_id, self.agent_id)
.await
}
// ==================== Convenience Methods ====================
@@ -976,7 +791,7 @@ impl Workspace {
/// comments, which the heartbeat runner treats as "effectively empty"
/// and skips the LLM call.
pub async fn heartbeat_checklist(&self) -> Result<Option<String>, WorkspaceError> {
match self.read_primary(paths::HEARTBEAT).await {
match self.read(paths::HEARTBEAT).await {
Ok(doc) => Ok(Some(doc.content)),
Err(WorkspaceError::DocumentNotFound { .. }) => Ok(Some(HEARTBEAT_SEED.to_string())),
Err(e) => Err(e),
@@ -984,29 +799,7 @@ impl Workspace {
}
/// Helper to read or create a file.
///
/// When multi-scope reads are configured, checks all read scopes before
/// creating. If the file exists in any scope, returns it. If not found in
/// any scope, creates it in the primary (write) scope.
///
/// **Important:** In multi-scope mode, the returned document may belong to
/// a secondary scope. Callers that intend to **write** to the document
/// (via `update_document(doc.id, ...)`) must NOT use this method — use
/// `storage.get_or_create_document_by_path(&self.user_id, ...)` instead
/// to guarantee writes target the primary scope. See `append_memory` for
/// the correct pattern.
async fn read_or_create(&self, path: &str) -> Result<MemoryDocument, WorkspaceError> {
if self.is_multi_scope() {
match self
.storage
.get_document_by_path_multi(&self.read_user_ids, self.agent_id, path)
.await
{
Ok(doc) => return Ok(doc),
Err(WorkspaceError::DocumentNotFound { .. }) => {}
Err(e) => return Err(e),
}
}
self.storage
.get_or_create_document_by_path(&self.user_id, self.agent_id, path)
.await
@@ -1018,18 +811,9 @@ impl Workspace {
///
/// This is for important facts, decisions, and preferences worth
/// remembering long-term.
///
/// Uses `get_or_create_document_by_path` with the primary `user_id`
/// instead of `self.memory()` to guarantee writes always target the
/// primary (write) scope. `self.memory()` delegates to `read_or_create`,
/// which in multi-scope mode may return a document owned by a secondary
/// scope; writing to that document by UUID would violate write isolation.
pub async fn append_memory(&self, entry: &str) -> Result<(), WorkspaceError> {
// Always get/create in the primary scope to preserve write isolation.
let doc = self
.storage
.get_or_create_document_by_path(&self.user_id, self.agent_id, paths::MEMORY)
.await?;
// Use double newline for memory entries (semantic separation)
let doc = self.memory().await?;
let new_content = if doc.content.is_empty() {
entry.to_string()
} else {
@@ -1121,16 +905,9 @@ impl Workspace {
// Safety net: if `profile_onboarding_completed` was already set (the
// LLM completed onboarding but forgot to delete BOOTSTRAP.md), skip
// injection to avoid repeating the first-run ritual.
//
// Identity and config files use read_primary() to prevent cross-scope
// bleed in multi-scope workspaces. Without this, a user with read access
// to other scopes could silently inherit another user's identity if their
// own copy is missing — the agent would present as the wrong person.
// Memory files (MEMORY.md, daily logs) intentionally use multi-scope
// read() since sharing memory across scopes is a feature.
let bootstrap_injected = if self.is_bootstrap_completed() {
if self
.read_primary(paths::BOOTSTRAP)
.read(paths::BOOTSTRAP)
.await
.is_ok_and(|d| !d.content.is_empty())
{
@@ -1140,7 +917,7 @@ impl Workspace {
);
}
false
} else if let Ok(doc) = self.read_primary(paths::BOOTSTRAP).await
} else if let Ok(doc) = self.read(paths::BOOTSTRAP).await
&& !doc.content.is_empty()
{
parts.push(format!("## First-Run Bootstrap\n\n{}", doc.content));
@@ -1149,8 +926,7 @@ impl Workspace {
false
};
// Load identity files in order of importance.
// These MUST use read_primary() — see comment above.
// Load identity files in order of importance
let identity_files = [
(paths::AGENTS, "## Agent Instructions"),
(paths::SOUL, "## Core Values"),
@@ -1159,7 +935,7 @@ impl Workspace {
];
for (path, header) in identity_files {
if let Ok(doc) = self.read_primary(path).await
if let Ok(doc) = self.read(path).await
&& !doc.content.is_empty()
{
parts.push(format!("{}\n\n{}", header, doc.content));
@@ -1168,8 +944,7 @@ impl Workspace {
// Tool notes: environment-specific guidance the agent or user has written.
// TOOLS.md does not control tool availability; it is guidance only.
// Uses read_primary() — tool config is per-user, not inherited.
if let Ok(doc) = self.read_primary(paths::TOOLS).await
if let Ok(doc) = self.read(paths::TOOLS).await
&& !doc.content.is_empty()
{
parts.push(format!("## Tool Notes\n\n{}", doc.content));
@@ -1460,8 +1235,6 @@ impl Workspace {
}
/// Search with custom configuration.
///
/// When multi-scope reads are configured, searches across all read scopes.
pub async fn search_with_config(
&self,
query: &str,
@@ -1481,46 +1254,15 @@ impl Workspace {
None
};
if self.is_multi_scope() {
let results = self
.storage
.hybrid_search_multi(
&self.read_user_ids,
self.agent_id,
query,
embedding.as_deref(),
&config,
)
.await?;
// Post-filter: exclude identity documents from secondary scopes.
// Collect document IDs that are identity paths in secondary scopes.
let mut excluded_doc_ids = std::collections::HashSet::new();
for result in &results {
if is_identity_path(&result.document_path) {
// Check if this document belongs to a secondary scope
match self.storage.get_document_by_id(result.document_id).await {
Ok(doc) if doc.user_id != self.user_id => {
excluded_doc_ids.insert(result.document_id);
}
_ => {}
}
}
}
Ok(results
.into_iter()
.filter(|r| !excluded_doc_ids.contains(&r.document_id))
.collect())
} else {
self.storage
.hybrid_search(
&self.user_id,
self.agent_id,
query,
embedding.as_deref(),
&config,
)
.await
}
self.storage
.hybrid_search(
&self.user_id,
self.agent_id,
query,
embedding.as_deref(),
&config,
)
.await
}
// ==================== Indexing ====================
@@ -1581,13 +1323,13 @@ impl Workspace {
// Check freshness BEFORE seeding identity files, otherwise the
// seeded files make the workspace look non-fresh and BOOTSTRAP.md
// never gets created.
let is_fresh_workspace = if self.read_primary(paths::BOOTSTRAP).await.is_ok() {
let is_fresh_workspace = if self.read(paths::BOOTSTRAP).await.is_ok() {
false // BOOTSTRAP already exists
} else {
let (agents_res, soul_res, user_res) = tokio::join!(
self.read_primary(paths::AGENTS),
self.read_primary(paths::SOUL),
self.read_primary(paths::USER),
self.read(paths::AGENTS),
self.read(paths::SOUL),
self.read(paths::USER),
);
matches!(agents_res, Err(WorkspaceError::DocumentNotFound { .. }))
&& matches!(soul_res, Err(WorkspaceError::DocumentNotFound { .. }))
@@ -1596,10 +1338,8 @@ impl Workspace {
let mut count = 0;
for (path, content) in seed_files {
// Skip files that already exist in the primary scope (never overwrite user edits).
// Uses read_primary to avoid false positives from secondary scopes —
// a file in another scope should not suppress seeding in this scope.
match self.read_primary(path).await {
// Skip files that already exist (never overwrite user edits)
match self.read(path).await {
Ok(_) => continue,
Err(WorkspaceError::DocumentNotFound { .. }) => {}
Err(e) => {
@@ -1620,8 +1360,7 @@ impl Workspace {
// may already have a profile from a previous install and doesn't need
// onboarding). This prevents existing users from getting a spurious
// first-run ritual after upgrading.
// Uses read_primary() to avoid false positives from secondary scopes.
let has_profile = self.read_primary(paths::PROFILE).await.is_ok_and(|d| {
let has_profile = self.read(paths::PROFILE).await.is_ok_and(|d| {
!d.content.trim().is_empty()
&& serde_json::from_str::<crate::profile::PsychographicProfile>(&d.content).is_ok()
});
@@ -2052,67 +1791,4 @@ mod seed_tests {
"BOOTSTRAP.md should NOT have been seeded with existing profile"
);
}
#[test]
fn test_default_single_scope() {
// Verify backward compatibility: default workspace has single read scope
// matching user_id.
let user_id = "alice";
let read_user_ids = [user_id.to_string()];
assert_eq!(read_user_ids.len(), 1);
assert_eq!(read_user_ids[0], user_id);
}
#[test]
fn test_additional_read_scopes() {
// Verify that additional read scopes are added correctly.
let user_id = "alice".to_string();
let mut read_user_ids = Vec::from([user_id.clone()]);
// Simulate with_additional_read_scopes logic
let scopes = ["shared", "team"];
for scope in scopes {
let s = scope.to_string();
if !read_user_ids.contains(&s) {
read_user_ids.push(s);
}
}
assert_eq!(read_user_ids.len(), 3);
assert_eq!(read_user_ids[0], "alice");
assert_eq!(read_user_ids[1], "shared");
assert_eq!(read_user_ids[2], "team");
}
#[test]
fn test_additional_read_scopes_dedup() {
// Verify that duplicate scopes are ignored.
let user_id = "alice".to_string();
let mut read_user_ids = Vec::from([user_id.clone()]);
let scopes = ["shared", "alice", "shared"];
for scope in scopes {
let s = scope.to_string();
if !read_user_ids.contains(&s) {
read_user_ids.push(s);
}
}
assert_eq!(read_user_ids.len(), 2);
assert_eq!(read_user_ids[0], "alice");
assert_eq!(read_user_ids[1], "shared");
}
#[test]
fn test_is_multi_scope_logic() {
// Test the multi-scope detection logic: > 1 means multi-scope
let single_count = 1_usize;
let multi_count = 2_usize;
// Single scope: not multi
assert!(single_count <= 1);
// Multi scope: is multi
assert!(multi_count > 1);
}
}
-199
View File
@@ -502,203 +502,4 @@ impl Repository {
})
.collect())
}
// ==================== Multi-scope search (optimized SQL) ====================
/// Hybrid search across multiple user scopes with efficient SQL.
///
/// Uses `user_id = ANY($1::text[])` instead of N separate queries.
pub async fn hybrid_search_multi(
&self,
user_ids: &[String],
agent_id: Option<Uuid>,
query: &str,
embedding: Option<&[f32]>,
config: &SearchConfig,
) -> Result<Vec<SearchResult>, WorkspaceError> {
let fts_results = if config.use_fts {
self.fts_search_multi(user_ids, agent_id, query, config.pre_fusion_limit)
.await?
} else {
Vec::new()
};
let vector_results = if config.use_vector {
if let Some(embedding) = embedding {
self.vector_search_multi(user_ids, agent_id, embedding, config.pre_fusion_limit)
.await?
} else {
Vec::new()
}
} else {
Vec::new()
};
Ok(fuse_results(fts_results, vector_results, config))
}
/// FTS search across multiple user scopes.
async fn fts_search_multi(
&self,
user_ids: &[String],
agent_id: Option<Uuid>,
query: &str,
limit: usize,
) -> Result<Vec<RankedResult>, WorkspaceError> {
let conn = self.conn().await?;
let rows = conn
.query(
r#"
SELECT c.id as chunk_id, c.document_id, d.path as document_path,
c.content,
ts_rank_cd(c.content_tsv, plainto_tsquery('english', $3)) as rank
FROM memory_chunks c
JOIN memory_documents d ON d.id = c.document_id
WHERE d.user_id = ANY($1::text[]) AND d.agent_id IS NOT DISTINCT FROM $2
AND c.content_tsv @@ plainto_tsquery('english', $3)
ORDER BY rank DESC
LIMIT $4
"#,
&[&user_ids, &agent_id, &query, &(limit as i64)],
)
.await
.map_err(|e| WorkspaceError::SearchFailed {
reason: format!("FTS multi-scope query failed: {}", e),
})?;
Ok(rows
.iter()
.enumerate()
.map(|(i, row)| RankedResult {
chunk_id: row.get("chunk_id"),
document_id: row.get("document_id"),
document_path: row.get("document_path"),
content: row.get("content"),
rank: (i + 1) as u32,
})
.collect())
}
/// Vector search across multiple user scopes.
async fn vector_search_multi(
&self,
user_ids: &[String],
agent_id: Option<Uuid>,
embedding: &[f32],
limit: usize,
) -> Result<Vec<RankedResult>, WorkspaceError> {
let conn = self.conn().await?;
let embedding_vec = Vector::from(embedding.to_vec());
let rows = conn
.query(
r#"
SELECT c.id as chunk_id, c.document_id, d.path as document_path,
c.content, 1 - (c.embedding <=> $3) as similarity
FROM memory_chunks c
JOIN memory_documents d ON d.id = c.document_id
WHERE d.user_id = ANY($1::text[]) AND d.agent_id IS NOT DISTINCT FROM $2
AND c.embedding IS NOT NULL
ORDER BY c.embedding <=> $3
LIMIT $4
"#,
&[&user_ids, &agent_id, &embedding_vec, &(limit as i64)],
)
.await
.map_err(|e| WorkspaceError::SearchFailed {
reason: format!("Vector multi-scope query failed: {}", e),
})?;
Ok(rows
.iter()
.enumerate()
.map(|(i, row)| RankedResult {
chunk_id: row.get("chunk_id"),
document_id: row.get("document_id"),
document_path: row.get("document_path"),
content: row.get("content"),
rank: (i + 1) as u32,
})
.collect())
}
/// List all file paths across multiple user scopes with a single query.
pub async fn list_all_paths_multi(
&self,
user_ids: &[String],
agent_id: Option<Uuid>,
) -> Result<Vec<String>, WorkspaceError> {
let conn = self.conn().await?;
let rows = conn
.query(
r#"
SELECT DISTINCT path FROM memory_documents
WHERE user_id = ANY($1::text[]) AND agent_id IS NOT DISTINCT FROM $2
ORDER BY path
"#,
&[&user_ids, &agent_id],
)
.await
.map_err(|e| WorkspaceError::SearchFailed {
reason: format!("List paths multi-scope failed: {}", e),
})?;
Ok(rows.iter().map(|row| row.get("path")).collect())
}
/// Get a document by path across multiple user scopes.
///
/// Returns the first match (ordered by the input user_ids priority).
pub async fn get_document_by_path_multi(
&self,
user_ids: &[String],
agent_id: Option<Uuid>,
path: &str,
) -> Result<MemoryDocument, WorkspaceError> {
let conn = self.conn().await?;
let row = conn
.query_opt(
r#"
SELECT id, user_id, agent_id, path, content,
created_at, updated_at, metadata
FROM memory_documents
WHERE user_id = ANY($1::text[]) AND agent_id IS NOT DISTINCT FROM $2 AND path = $3
ORDER BY array_position($1::text[], user_id)
LIMIT 1
"#,
&[&user_ids, &agent_id, &path],
)
.await
.map_err(|e| WorkspaceError::SearchFailed {
reason: format!("get_document_by_path_multi failed: {}", e),
})?;
match row {
Some(row) => Ok(self.row_to_document(&row)),
None => Err(WorkspaceError::DocumentNotFound {
doc_type: path.to_string(),
user_id: format!("[{}]", user_ids.join(", ")),
}),
}
}
/// List directory contents across multiple user scopes.
///
/// Iterates per scope and merges results. A future migration could add an
/// optimised SQL function, at which point this method can call it directly.
pub async fn list_directory_multi(
&self,
user_ids: &[String],
agent_id: Option<Uuid>,
directory: &str,
) -> Result<Vec<WorkspaceEntry>, 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))
}
}
@@ -1,249 +0,0 @@
"""OAuth URL parameter validation e2e tests.
Tests for bug #992: Google OAuth URL broken when initiated from Telegram.
Specifically verifies that OAuth query parameters are correctly formatted:
- "client_id" (with underscore) NOT "clientid" (without underscore)
- All standard OAuth parameters are present and correctly encoded
- URLs are consistent across channels (web, Telegram, etc.)
The test verifies:
1. OAuth URL is generated with correct parameters
2. URL works with the OAuth provider (Google)
3. Extra parameters (access_type, prompt) are preserved
"""
from urllib.parse import parse_qs, urlparse
import pytest
from helpers import api_post, api_get
async def _extract_oauth_params(auth_url: str) -> dict:
"""Extract and validate OAuth query parameters from auth_url.
Returns dict with parsed parameters:
{
'client_id': '...',
'redirect_uri': '...',
'response_type': 'code',
'scope': '...',
'state': '...',
'access_type': '...',
'prompt': '...',
...
}
"""
parsed = urlparse(auth_url)
qs = parse_qs(parsed.query)
# Convert lists to single values for easier testing
params = {k: v[0] if len(v) > 0 else v for k, v in qs.items()}
return params
async def _get_extension(ironclaw_server, name):
"""Get a specific extension from the extensions list, or None."""
r = await api_get(ironclaw_server, "/api/extensions")
for ext in r.json().get("extensions", []):
if ext["name"] == name:
return ext
return None
@pytest.fixture
async def installed_gmail(ironclaw_server):
"""Installs the 'gmail' extension before a test and removes it after.
This fixture handles the setup and teardown of the Gmail extension,
ensuring a clean state for each test.
"""
# Ensure Gmail is not installed before test
ext = await _get_extension(ironclaw_server, "gmail")
if ext:
r = await api_post(ironclaw_server, "/api/extensions/gmail/remove", timeout=30)
assert r.status_code == 200
# Install Gmail
r = await api_post(
ironclaw_server,
"/api/extensions/install",
json={"name": "gmail"},
timeout=180,
)
assert r.status_code == 200, f"Gmail install failed: {r.text}"
assert r.json().get("success") is True, f"Install failed: {r.json().get('message', '')}"
yield
# Teardown: remove gmail
r = await api_post(ironclaw_server, "/api/extensions/gmail/remove", timeout=30)
assert r.status_code == 200, f"Gmail removal failed: {r.text}"
@pytest.fixture
async def auth_url(ironclaw_server, installed_gmail):
"""Generate and return an OAuth auth URL.
Requires Gmail to be installed (depends on installed_gmail fixture).
"""
r = await api_post(
ironclaw_server,
"/api/extensions/gmail/setup",
json={"secrets": {}},
timeout=30,
)
assert r.status_code == 200
data = r.json()
assert data.get("success") is True, f"Setup failed: {data.get('message', '')}"
url = data.get("auth_url")
assert url is not None, f"Expected auth_url in response: {data}"
assert "accounts.google.com" in url, f"auth_url should point to Google: {url}"
return url
@pytest.fixture
async def oauth_params(auth_url):
"""Extract and return OAuth parameters from auth_url.
Depends on auth_url fixture.
"""
return await _extract_oauth_params(auth_url)
# ─ OAuth URL parameter validation tests ────────────────────────────────
async def test_oauth_url_has_client_id_not_clientid(oauth_params, auth_url):
"""Verify OAuth URL has 'client_id' (with underscore), NOT 'clientid'.
Bug #992: Ensure the parameter name is correct across all channels.
"""
params = oauth_params
# The bug: "clientid" appears instead of "client_id"
# Verify the CORRECT parameter name exists
assert "client_id" in params, (
f"OAuth URL missing 'client_id' parameter. "
f"URL: {auth_url}\nParams: {params}"
)
assert params["client_id"], "client_id should have a value"
# Verify the INCORRECT parameter name does NOT exist
assert "clientid" not in params, (
f"OAuth URL should NOT have 'clientid' (without underscore). "
f"Bug #992: URL: {auth_url}\nParams: {params}"
)
async def test_oauth_url_has_required_parameters(oauth_params):
"""Verify all required OAuth 2.0 parameters are present."""
params = oauth_params
# Required OAuth 2.0 parameters
required = ["client_id", "response_type", "redirect_uri", "scope", "state"]
for param in required:
assert param in params, (
f"Missing required OAuth parameter: {param}. "
f"Params: {params}"
)
assert params[param], f"Parameter '{param}' should have a non-empty value"
# Validate specific values
assert params["response_type"] == "code", "Should use authorization_code flow"
assert "oauth" in params["redirect_uri"], "Redirect URI should be an OAuth callback"
async def test_oauth_url_has_extra_params(oauth_params):
"""Verify extra_params from capabilities.json are included."""
params = oauth_params
# Google-specific extra_params from gmail-tool.capabilities.json
assert "access_type" in params, (
"Should include 'access_type' from extra_params"
)
assert params["access_type"] == "offline", (
"access_type should be 'offline' for Gmail"
)
assert "prompt" in params, (
"Should include 'prompt' from extra_params"
)
assert params["prompt"] == "consent", (
"prompt should be 'consent' for Gmail"
)
async def test_oauth_url_is_valid_google_oauth(auth_url):
"""Verify the URL is a valid Google OAuth 2.0 authorization URL."""
# Verify scheme and host
parsed = urlparse(auth_url)
assert parsed.scheme == "https", "OAuth URL must use HTTPS"
assert "accounts.google.com" in parsed.netloc, "Must be Google's OAuth endpoint"
assert parsed.path == "/o/oauth2/v2/auth", "Must use Google OAuth 2.0 endpoint"
async def test_oauth_url_state_is_unique(ironclaw_server, installed_gmail, oauth_params, auth_url):
"""Verify CSRF state is present and unique per request."""
# Get a new OAuth URL
r = await api_post(
ironclaw_server,
"/api/extensions/gmail/setup",
json={"secrets": {}},
timeout=30,
)
assert r.status_code == 200
new_auth_url = r.json().get("auth_url")
assert new_auth_url is not None
# Extract state from both URLs
original_params = oauth_params
new_params = await _extract_oauth_params(new_auth_url)
original_state = original_params.get("state")
new_state = new_params.get("state")
assert original_state is not None, "Should have state parameter"
assert new_state is not None, "New request should have state parameter"
assert original_state != new_state, (
"CSRF state should be unique per request (for security)"
)
async def test_oauth_url_escaping(auth_url):
"""Verify URL query parameters are properly escaped."""
# Verify special characters in values are URL-encoded
# For example, scopes contain spaces which should be %20
assert "%20" in auth_url or "+" in auth_url or "%2B" in auth_url or " " not in auth_url, (
"OAuth URL should properly encode special characters in parameters"
)
# ─ Telegram-specific tests (when Telegram channel is available) ──────────
class TestOAuthURLViaTelegram:
"""Test OAuth URL generation specifically via Telegram channel.
These tests would verify that the same OAuth URL works correctly when
transmitted through the Telegram WASM channel (as opposed to web gateway).
Currently marked as xfail pending Telegram channel setup in E2E tests.
"""
@pytest.mark.skip(reason="Telegram channel E2E setup not yet implemented")
async def test_telegram_oauth_url_has_correct_parameters(self):
"""Verify OAuth URL sent via Telegram has correct parameter names."""
# This test would:
# 1. Send a message via Telegram that triggers OAuth
# 2. Capture the status update sent to Telegram
# 3. Extract the auth_url from the message
# 4. Verify it has "client_id" not "clientid"
pass
@pytest.mark.skip(reason="Telegram channel E2E setup not yet implemented")
async def test_telegram_oauth_url_can_be_regenerated(self):
"""Verify OAuth URL can be regenerated when requested via Telegram."""
# This test would verify that the bug #992 symptom
# "URL cannot be regenerated when asked" is fixed.
# If the URL is cached incorrectly, regeneration would fail.
pass
-195
View File
@@ -1,195 +0,0 @@
//! Tests for identity file scope isolation in multi-scope workspaces.
//!
//! When a workspace has multiple read scopes (e.g., Andrew can read from
//! "andrew", "grace", "household"), identity files (SOUL.md, USER.md,
//! IDENTITY.md, AGENTS.md) must ONLY come from the primary scope.
//!
//! Multi-scope reads are designed for memory sharing (MEMORY.md, daily logs),
//! not identity inheritance. Silently inheriting identity from another scope
//! is a correctness and security issue — the agent would present itself as
//! the wrong user.
//!
//! These tests verify that:
//! 1. Identity files are read from primary scope only
//! 2. If the primary scope's identity file is missing, it's absent from the
//! system prompt — never falls back to another scope
//! 3. Memory files (MEMORY.md) still benefit from multi-scope reads
#![cfg(feature = "libsql")]
use std::sync::Arc;
use ironclaw::db::Database;
use ironclaw::db::libsql::LibSqlBackend;
use ironclaw::workspace::{Workspace, paths};
async fn setup() -> (Arc<dyn Database>, tempfile::TempDir) {
let dir = tempfile::tempdir().expect("create temp dir");
let db_path = dir.path().join("test.db");
let backend = LibSqlBackend::new_local(&db_path).await.expect("create db");
backend.run_migrations().await.expect("run migrations");
let db: Arc<dyn Database> = Arc::new(backend);
(db, dir)
}
/// Seed a document into a specific user's workspace scope.
async fn seed(db: &Arc<dyn Database>, user_id: &str, path: &str, content: &str) {
let ws = Workspace::new_with_db(user_id, db.clone());
ws.write(path, content)
.await
.unwrap_or_else(|e| panic!("Failed to seed {path} for {user_id}: {e}"));
}
// ─── Test 1: Primary scope identity appears in system prompt ───────────
#[tokio::test]
async fn system_prompt_uses_primary_scope_identity() {
let (db, _dir) = setup().await;
// Seed Alice's identity files in her own scope
seed(&db, "alice", paths::SOUL, "Alice is kind and curious.").await;
seed(
&db,
"alice",
paths::USER,
"You are talking to Alice, a software engineer.",
)
.await;
// Seed Bob's identity files in his scope
seed(&db, "bob", paths::SOUL, "Bob is analytical and precise.").await;
seed(
&db,
"bob",
paths::USER,
"You are talking to Bob, a marine biologist.",
)
.await;
// Create Alice's workspace WITH multi-scope reads including Bob
let ws = Workspace::new_with_db("alice", db.clone())
.with_additional_read_scopes(vec!["bob".to_string()]);
let prompt = ws
.system_prompt_for_context(false)
.await
.expect("system_prompt_for_context failed");
// Alice's identity must appear
assert!(
prompt.contains("Alice is kind and curious"),
"Primary scope SOUL.md should appear in system prompt.\nPrompt:\n{prompt}"
);
assert!(
prompt.contains("Alice, a software engineer"),
"Primary scope USER.md should appear in system prompt.\nPrompt:\n{prompt}"
);
// Bob's identity must NOT appear
assert!(
!prompt.contains("Bob is analytical"),
"Secondary scope SOUL.md must NOT appear in system prompt.\nPrompt:\n{prompt}"
);
assert!(
!prompt.contains("Bob, a marine biologist"),
"Secondary scope USER.md must NOT appear in system prompt.\nPrompt:\n{prompt}"
);
}
// ─── Test 2: Missing primary identity does NOT fall back to other scope ─
#[tokio::test]
async fn missing_primary_identity_does_not_fallback_to_other_scope() {
let (db, _dir) = setup().await;
// Only seed Bob's identity — Alice has no identity files
seed(&db, "bob", paths::SOUL, "Bob is analytical and precise.").await;
seed(
&db,
"bob",
paths::USER,
"You are talking to Bob, a marine biologist.",
)
.await;
// Create Alice's workspace with multi-scope reads including Bob
let ws = Workspace::new_with_db("alice", db.clone())
.with_additional_read_scopes(vec!["bob".to_string()]);
let prompt = ws
.system_prompt_for_context(false)
.await
.expect("system_prompt_for_context failed");
// Bob's identity must NOT appear — Alice's missing identity should stay missing,
// not silently inherit from Bob's scope
assert!(
!prompt.contains("Bob"),
"When primary scope identity is missing, must NOT fall back to secondary scope.\n\
This would cause the agent to present itself as the wrong user.\nPrompt:\n{prompt}"
);
}
// ─── Test 3: MEMORY.md still benefits from multi-scope reads ────────────
#[tokio::test]
async fn memory_files_still_use_multi_scope_reads() {
let (db, _dir) = setup().await;
// Seed shared memory in the "shared" scope (not Alice's primary)
seed(
&db,
"shared",
paths::MEMORY,
"Shared grocery list: milk, eggs, bread.",
)
.await;
// Create Alice's workspace with read access to shared scope
let ws = Workspace::new_with_db("alice", db.clone())
.with_additional_read_scopes(vec!["shared".to_string()]);
let prompt = ws
.system_prompt_for_context(false)
.await
.expect("system_prompt_for_context failed");
// Shared memory SHOULD appear — multi-scope reads are correct for memory
assert!(
prompt.contains("grocery list"),
"MEMORY.md should still use multi-scope reads.\nPrompt:\n{prompt}"
);
}
// ─── Test 4: All identity files are scope-isolated ──────────────────────
#[tokio::test]
async fn all_identity_files_are_scope_isolated() {
let (db, _dir) = setup().await;
// Seed identity files ONLY in the "other" scope, not in Alice's
seed(&db, "other", paths::AGENTS, "You are Other's agent.").await;
seed(&db, "other", paths::SOUL, "Other's soul values.").await;
seed(&db, "other", paths::USER, "You are talking to Other.").await;
seed(&db, "other", paths::IDENTITY, "Other's identity.").await;
// Also seed BOOTSTRAP.md and TOOLS.md in other scope
seed(&db, "other", "BOOTSTRAP.md", "Other's bootstrap.").await;
seed(&db, "other", "TOOLS.md", "Other's tool notes.").await;
// Create Alice's workspace with read access to "other"
let ws = Workspace::new_with_db("alice", db.clone())
.with_additional_read_scopes(vec!["other".to_string()]);
let prompt = ws
.system_prompt_for_context(false)
.await
.expect("system_prompt_for_context failed");
// None of Other's identity/config files should appear
assert!(
!prompt.contains("Other"),
"No identity or config files from secondary scope should appear.\n\
Every identity file (AGENTS.md, SOUL.md, USER.md, IDENTITY.md, \
BOOTSTRAP.md, TOOLS.md) must read from primary scope only.\nPrompt:\n{prompt}"
);
}
-451
View File
@@ -1,451 +0,0 @@
#![cfg(feature = "libsql")]
//! Integration tests for multi-scope workspace reads using file-backed libSQL.
//!
//! Guards the PR2 contract: workspaces can read from multiple user scopes
//! while writes remain isolated to the primary scope.
use std::sync::Arc;
use ironclaw::db::Database;
use ironclaw::db::libsql::LibSqlBackend;
use ironclaw::workspace::Workspace;
async fn setup() -> (Arc<dyn Database>, tempfile::TempDir) {
let dir = tempfile::tempdir().expect("create temp dir");
let db_path = dir.path().join("test.db");
let backend = LibSqlBackend::new_local(&db_path).await.expect("create db");
backend.run_migrations().await.expect("run migrations");
let db: Arc<dyn Database> = Arc::new(backend);
(db, dir)
}
#[tokio::test]
async fn read_across_scopes() {
let (db, _dir) = setup().await;
// Write docs as the "shared" user
let ws_shared = Workspace::new_with_db("shared", Arc::clone(&db));
ws_shared
.write("docs/team-standup.md", "Team standup notes from Monday")
.await
.expect("shared write failed");
// Alice's workspace with "shared" as an additional read scope
let ws_alice = Workspace::new_with_db("alice", Arc::clone(&db))
.with_additional_read_scopes(vec!["shared".to_string()]);
// Alice can read shared docs
let doc = ws_alice
.read("docs/team-standup.md")
.await
.expect("cross-scope read failed");
assert_eq!(doc.content, "Team standup notes from Monday");
}
#[tokio::test]
async fn write_stays_in_primary_scope() {
let (db, _dir) = setup().await;
// Alice has "shared" as a read scope
let ws_alice = Workspace::new_with_db("alice", Arc::clone(&db))
.with_additional_read_scopes(vec!["shared".to_string()]);
// Alice writes a personal note
ws_alice
.write("notes/personal.md", "Alice's private note")
.await
.expect("alice write failed");
// The "shared" workspace should NOT see Alice's note
let ws_shared = Workspace::new_with_db("shared", Arc::clone(&db));
let result = ws_shared.read("notes/personal.md").await;
assert!(result.is_err(), "Shared scope should not see Alice's note");
}
#[tokio::test]
async fn list_paths_merges_across_scopes() {
let (db, _dir) = setup().await;
// Write as alice
let ws_alice_plain = Workspace::new_with_db("alice", Arc::clone(&db));
ws_alice_plain
.write("notes/personal.md", "My notes")
.await
.expect("alice write failed");
// Write as shared
let ws_shared = Workspace::new_with_db("shared", Arc::clone(&db));
ws_shared
.write("docs/shared-doc.md", "Shared document")
.await
.expect("shared write failed");
// Alice with multi-scope should see both
let ws_alice = Workspace::new_with_db("alice", Arc::clone(&db))
.with_additional_read_scopes(vec!["shared".to_string()]);
let all_paths = ws_alice.list_all().await.expect("list_all failed");
assert!(
all_paths.contains(&"notes/personal.md".to_string()),
"Should contain alice's note: {:?}",
all_paths
);
assert!(
all_paths.contains(&"docs/shared-doc.md".to_string()),
"Should contain shared doc: {:?}",
all_paths
);
}
#[tokio::test]
async fn list_directory_merges_across_scopes() {
let (db, _dir) = setup().await;
// Alice writes to docs/
let ws_alice_plain = Workspace::new_with_db("alice", Arc::clone(&db));
ws_alice_plain
.write("docs/alice-doc.md", "Alice's doc")
.await
.expect("alice write failed");
// Shared writes to docs/
let ws_shared = Workspace::new_with_db("shared", Arc::clone(&db));
ws_shared
.write("docs/shared-doc.md", "Shared doc")
.await
.expect("shared write failed");
// Alice with multi-scope lists docs/
let ws_alice = Workspace::new_with_db("alice", Arc::clone(&db))
.with_additional_read_scopes(vec!["shared".to_string()]);
let entries = ws_alice.list("docs").await.expect("list failed");
let paths: Vec<&str> = entries.iter().map(|e| e.path.as_str()).collect();
assert!(
paths.contains(&"docs/alice-doc.md"),
"Should contain alice's doc: {:?}",
paths
);
assert!(
paths.contains(&"docs/shared-doc.md"),
"Should contain shared doc: {:?}",
paths
);
}
#[tokio::test]
async fn search_spans_scopes() {
let (db, _dir) = setup().await;
// Write searchable content in shared scope
let ws_shared = Workspace::new_with_db("shared", Arc::clone(&db));
ws_shared
.write(
"docs/architecture.md",
"The microservice architecture uses gRPC for inter-service communication",
)
.await
.expect("shared write failed");
// Write searchable content in alice scope
let ws_alice_plain = Workspace::new_with_db("alice", Arc::clone(&db));
ws_alice_plain
.write("notes/ideas.md", "Consider switching to GraphQL federation")
.await
.expect("alice write failed");
// Alice with multi-scope searches
let ws_alice = Workspace::new_with_db("alice", Arc::clone(&db))
.with_additional_read_scopes(vec!["shared".to_string()]);
// Search for content in the shared scope
let results = ws_alice
.search("microservice architecture gRPC", 10)
.await
.expect("search failed");
assert!(!results.is_empty(), "Should find results from shared scope");
}
#[tokio::test]
async fn read_priority_primary_first() {
let (db, _dir) = setup().await;
// Write same path in both scopes
let ws_shared = Workspace::new_with_db("shared", Arc::clone(&db));
ws_shared
.write("config/settings.md", "Shared settings v1")
.await
.expect("shared write failed");
let ws_alice_plain = Workspace::new_with_db("alice", Arc::clone(&db));
ws_alice_plain
.write("config/settings.md", "Alice's settings override")
.await
.expect("alice write failed");
// Alice with multi-scope should get her own version (primary scope wins)
let ws_alice = Workspace::new_with_db("alice", Arc::clone(&db))
.with_additional_read_scopes(vec!["shared".to_string()]);
let doc = ws_alice
.read("config/settings.md")
.await
.expect("read failed");
assert_eq!(
doc.content, "Alice's settings override",
"Primary scope should take priority"
);
}
#[tokio::test]
async fn exists_spans_scopes() {
let (db, _dir) = setup().await;
// Write a doc as "shared"
let ws_shared = Workspace::new_with_db("shared", Arc::clone(&db));
ws_shared
.write("docs/shared-only.md", "Shared content")
.await
.expect("shared write failed");
// Alice without multi-scope should NOT see it
let ws_alice_plain = Workspace::new_with_db("alice", Arc::clone(&db));
assert!(
!ws_alice_plain
.exists("docs/shared-only.md")
.await
.expect("exists failed"),
"Alice without multi-scope should not see shared doc"
);
// Alice with multi-scope should see it
let ws_alice = Workspace::new_with_db("alice", Arc::clone(&db))
.with_additional_read_scopes(vec!["shared".to_string()]);
assert!(
ws_alice
.exists("docs/shared-only.md")
.await
.expect("exists failed"),
"Alice with multi-scope should see shared doc"
);
}
#[tokio::test]
async fn append_stays_in_primary_scope() {
let (db, _dir) = setup().await;
// Write a document as "shared"
let ws_shared = Workspace::new_with_db("shared", Arc::clone(&db));
ws_shared
.write("notes/log.md", "shared original content")
.await
.expect("shared write failed");
// Alice has "shared" as a read scope and appends to the same path
let ws_alice = Workspace::new_with_db("alice", Arc::clone(&db))
.with_additional_read_scopes(vec!["shared".to_string()]);
ws_alice
.append("notes/log.md", "alice appended line")
.await
.expect("alice append failed");
// Shared document must be unchanged (write isolation)
let shared_doc = ws_shared
.read("notes/log.md")
.await
.expect("shared read failed");
assert_eq!(
shared_doc.content, "shared original content",
"Append must not modify the secondary scope's document"
);
// Alice should have her own copy with the appended content
let ws_alice_plain = Workspace::new_with_db("alice", Arc::clone(&db));
let alice_doc = ws_alice_plain
.read("notes/log.md")
.await
.expect("alice read failed");
assert_eq!(
alice_doc.content, "alice appended line",
"Append should create a new document in alice's scope"
);
}
#[tokio::test]
async fn append_memory_stays_in_primary_scope() {
let (db, _dir) = setup().await;
// Write MEMORY.md as "shared"
let ws_shared = Workspace::new_with_db("shared", Arc::clone(&db));
ws_shared
.write("MEMORY.md", "shared memory baseline")
.await
.expect("shared write failed");
// Alice has "shared" as a read scope and appends a memory entry
let ws_alice = Workspace::new_with_db("alice", Arc::clone(&db))
.with_additional_read_scopes(vec!["shared".to_string()]);
ws_alice
.append_memory("alice remembers this")
.await
.expect("alice append_memory failed");
// Shared MEMORY.md must be unchanged
let shared_doc = ws_shared
.read("MEMORY.md")
.await
.expect("shared read failed");
assert_eq!(
shared_doc.content, "shared memory baseline",
"append_memory must not modify the secondary scope's document"
);
// Alice should have her own MEMORY.md
let ws_alice_plain = Workspace::new_with_db("alice", Arc::clone(&db));
let alice_doc = ws_alice_plain
.read("MEMORY.md")
.await
.expect("alice read failed");
assert_eq!(
alice_doc.content, "alice remembers this",
"append_memory should create in alice's scope"
);
}
// ==================== Identity isolation tests ====================
#[tokio::test]
async fn identity_files_not_readable_from_secondary_scope() {
let (db, _dir) = setup().await;
let ws_other = Workspace::new_with_db("other-user", Arc::clone(&db));
ws_other
.write("IDENTITY.md", "I am the other user")
.await
.expect("write failed");
ws_other
.write("SOUL.md", "Other user soul overlay")
.await
.expect("write failed");
ws_other
.write("USER.md", "Other user profile")
.await
.expect("write failed");
ws_other
.write("AGENTS.md", "Other user agent config")
.await
.expect("write failed");
let ws_primary = Workspace::new_with_db("primary", Arc::clone(&db))
.with_additional_read_scopes(vec!["other-user".to_string()]);
for path in &["IDENTITY.md", "SOUL.md", "USER.md", "AGENTS.md"] {
let result = ws_primary.read(path).await;
assert!(
result.is_err(),
"Primary should NOT read other user's {} via secondary scope",
path
);
}
}
#[tokio::test]
async fn identity_files_not_in_search_from_secondary_scope() {
let (db, _dir) = setup().await;
let ws_other = Workspace::new_with_db("other-user", Arc::clone(&db));
ws_other
.write("SOUL.md", "Other user loves xylophone music passionately")
.await
.expect("write failed");
ws_other
.write(
"notes/music.md",
"Other user played xylophone at the concert",
)
.await
.expect("write failed");
let ws_primary = Workspace::new_with_db("primary", Arc::clone(&db))
.with_additional_read_scopes(vec!["other-user".to_string()]);
let results = ws_primary
.search("xylophone", 10)
.await
.expect("search failed");
let has_concert = results.iter().any(|r| r.content.contains("concert"));
assert!(
has_concert,
"Should find non-identity content from secondary scope"
);
let has_soul = results.iter().any(|r| r.content.contains("passionately"));
assert!(
!has_soul,
"SOUL.md content from secondary scope should not appear in search results"
);
}
#[tokio::test]
async fn identity_files_not_in_list_from_secondary_scope() {
let (db, _dir) = setup().await;
let ws_other = Workspace::new_with_db("other-user", Arc::clone(&db));
ws_other
.write("IDENTITY.md", "I am the other user")
.await
.expect("write failed");
ws_other
.write("notes/shared-note.md", "A shared note")
.await
.expect("write failed");
let ws_primary = Workspace::new_with_db("primary", Arc::clone(&db))
.with_additional_read_scopes(vec!["other-user".to_string()]);
let paths = ws_primary.list_all().await.expect("list failed");
assert!(
!paths.contains(&"IDENTITY.md".to_string()),
"IDENTITY.md from secondary scope should not appear"
);
assert!(
paths.contains(&"notes/shared-note.md".to_string()),
"Non-identity files should be listed"
);
}
#[tokio::test]
async fn empty_read_scopes_reads_primary_only() {
let (db, _dir) = setup().await;
let ws_shared = Workspace::new_with_db("shared", Arc::clone(&db));
ws_shared
.write("docs/note.md", "Shared note")
.await
.expect("write failed");
let ws_primary =
Workspace::new_with_db("primary", Arc::clone(&db)).with_additional_read_scopes(vec![]);
let result = ws_primary.read("docs/note.md").await;
assert!(
result.is_err(),
"Empty read scopes should not grant cross-scope access"
);
}
#[tokio::test]
async fn duplicate_read_scopes_handled() {
let (db, _dir) = setup().await;
let ws_shared = Workspace::new_with_db("shared", Arc::clone(&db));
ws_shared
.write("docs/note.md", "One note")
.await
.expect("write failed");
let ws_primary = Workspace::new_with_db("primary", Arc::clone(&db))
.with_additional_read_scopes(vec!["shared".to_string(), "shared".to_string()]);
let doc = ws_primary.read("docs/note.md").await.expect("read failed");
assert_eq!(doc.content, "One note");
}
-330
View File
@@ -407,333 +407,3 @@ async fn test_workspace_system_prompt() {
cleanup_user(&pool, user_id).await;
}
// ── Multi-scope workspace read tests ──────────────────────────────────
//
// These exercise the PostgreSQL-optimized `_multi` query paths
// (repository.rs) that the libSQL backend covers via default trait impls.
#[tokio::test]
async fn test_multi_scope_read_across_scopes() {
let pool = get_pool();
if try_connect(&pool).await.is_none() {
return;
}
let shared_id = "ms_shared_read";
let alice_id = "ms_alice_read";
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
// Write a doc as "shared"
let ws_shared = Workspace::new(shared_id, pool.clone());
ws_shared
.write("docs/team-standup.md", "Team standup notes from Monday")
.await
.expect("shared write failed");
// Alice with "shared" as an additional read scope
let ws_alice = Workspace::new(alice_id, pool.clone())
.with_additional_read_scopes(vec![shared_id.to_string()]);
let doc = ws_alice
.read("docs/team-standup.md")
.await
.expect("cross-scope read failed");
assert_eq!(doc.content, "Team standup notes from Monday");
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
}
#[tokio::test]
async fn test_multi_scope_write_stays_in_primary() {
let pool = get_pool();
if try_connect(&pool).await.is_none() {
return;
}
let shared_id = "ms_shared_write";
let alice_id = "ms_alice_write";
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
let ws_alice = Workspace::new(alice_id, pool.clone())
.with_additional_read_scopes(vec![shared_id.to_string()]);
ws_alice
.write("notes/personal.md", "Alice's private note")
.await
.expect("alice write failed");
// Shared workspace should NOT see Alice's note
let ws_shared = Workspace::new(shared_id, pool.clone());
let result = ws_shared.read("notes/personal.md").await;
assert!(result.is_err(), "Shared scope should not see Alice's note");
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
}
#[tokio::test]
async fn test_multi_scope_list_all_merges() {
let pool = get_pool();
if try_connect(&pool).await.is_none() {
return;
}
let shared_id = "ms_shared_list";
let alice_id = "ms_alice_list";
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
// Write as alice (plain, no multi-scope)
let ws_alice_plain = Workspace::new(alice_id, pool.clone());
ws_alice_plain
.write("notes/personal.md", "My notes")
.await
.expect("alice write failed");
// Write as shared
let ws_shared = Workspace::new(shared_id, pool.clone());
ws_shared
.write("docs/shared-doc.md", "Shared document")
.await
.expect("shared write failed");
// Alice with multi-scope should see both
let ws_alice = Workspace::new(alice_id, pool.clone())
.with_additional_read_scopes(vec![shared_id.to_string()]);
let all_paths = ws_alice.list_all().await.expect("list_all failed");
assert!(
all_paths.contains(&"notes/personal.md".to_string()),
"Should contain alice's note: {:?}",
all_paths
);
assert!(
all_paths.contains(&"docs/shared-doc.md".to_string()),
"Should contain shared doc: {:?}",
all_paths
);
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
}
#[tokio::test]
async fn test_multi_scope_list_directory_merges() {
let pool = get_pool();
if try_connect(&pool).await.is_none() {
return;
}
let shared_id = "ms_shared_dir";
let alice_id = "ms_alice_dir";
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
let ws_alice_plain = Workspace::new(alice_id, pool.clone());
ws_alice_plain
.write("docs/alice-doc.md", "Alice's doc")
.await
.expect("alice write failed");
let ws_shared = Workspace::new(shared_id, pool.clone());
ws_shared
.write("docs/shared-doc.md", "Shared doc")
.await
.expect("shared write failed");
let ws_alice = Workspace::new(alice_id, pool.clone())
.with_additional_read_scopes(vec![shared_id.to_string()]);
let entries = ws_alice.list("docs").await.expect("list failed");
let paths: Vec<&str> = entries.iter().map(|e| e.path.as_str()).collect();
assert!(
paths.contains(&"docs/alice-doc.md"),
"Should contain alice's doc: {:?}",
paths
);
assert!(
paths.contains(&"docs/shared-doc.md"),
"Should contain shared doc: {:?}",
paths
);
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
}
#[tokio::test]
async fn test_multi_scope_read_priority_primary_first() {
let pool = get_pool();
if try_connect(&pool).await.is_none() {
return;
}
let shared_id = "ms_shared_prio";
let alice_id = "ms_alice_prio";
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
// Write same path in both scopes
let ws_shared = Workspace::new(shared_id, pool.clone());
ws_shared
.write("config/settings.md", "Shared settings v1")
.await
.expect("shared write failed");
let ws_alice_plain = Workspace::new(alice_id, pool.clone());
ws_alice_plain
.write("config/settings.md", "Alice's settings override")
.await
.expect("alice write failed");
// Alice with multi-scope should get her own version (primary scope wins)
let ws_alice = Workspace::new(alice_id, pool.clone())
.with_additional_read_scopes(vec![shared_id.to_string()]);
let doc = ws_alice
.read("config/settings.md")
.await
.expect("read failed");
assert_eq!(
doc.content, "Alice's settings override",
"Primary scope should take priority"
);
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
}
#[tokio::test]
async fn test_multi_scope_exists_spans_scopes() {
let pool = get_pool();
if try_connect(&pool).await.is_none() {
return;
}
let shared_id = "ms_shared_exists";
let alice_id = "ms_alice_exists";
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
let ws_shared = Workspace::new(shared_id, pool.clone());
ws_shared
.write("docs/shared-only.md", "Shared content")
.await
.expect("shared write failed");
// Alice without multi-scope should NOT see it
let ws_alice_plain = Workspace::new(alice_id, pool.clone());
assert!(
!ws_alice_plain
.exists("docs/shared-only.md")
.await
.expect("exists failed"),
"Alice without multi-scope should not see shared doc"
);
// Alice with multi-scope should see it
let ws_alice = Workspace::new(alice_id, pool.clone())
.with_additional_read_scopes(vec![shared_id.to_string()]);
assert!(
ws_alice
.exists("docs/shared-only.md")
.await
.expect("exists failed"),
"Alice with multi-scope should see shared doc"
);
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
}
#[tokio::test]
async fn test_multi_scope_search_spans_scopes() {
let pool = get_pool();
if try_connect(&pool).await.is_none() {
return;
}
let shared_id = "ms_shared_search";
let alice_id = "ms_alice_search";
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
let ws_shared = Workspace::new(shared_id, pool.clone());
ws_shared
.write(
"docs/architecture.md",
"The microservice architecture uses gRPC for inter-service communication",
)
.await
.expect("shared write failed");
let ws_alice_plain = Workspace::new(alice_id, pool.clone());
ws_alice_plain
.write("notes/ideas.md", "Consider switching to GraphQL federation")
.await
.expect("alice write failed");
let ws_alice = Workspace::new(alice_id, pool.clone())
.with_additional_read_scopes(vec![shared_id.to_string()]);
// Search for content in the shared scope
let results = ws_alice
.search_with_config(
"microservice gRPC architecture",
SearchConfig::default().fts_only(),
)
.await
.expect("search failed");
assert!(!results.is_empty(), "Should find results from shared scope");
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
}
#[tokio::test]
async fn test_multi_scope_append_stays_in_primary() {
let pool = get_pool();
if try_connect(&pool).await.is_none() {
return;
}
let shared_id = "ms_shared_append";
let alice_id = "ms_alice_append";
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
// Write a document as "shared"
let ws_shared = Workspace::new(shared_id, pool.clone());
ws_shared
.write("notes/log.md", "shared original content")
.await
.expect("shared write failed");
// Alice has "shared" as a read scope and appends to the same path
let ws_alice = Workspace::new(alice_id, pool.clone())
.with_additional_read_scopes(vec![shared_id.to_string()]);
ws_alice
.append("notes/log.md", "alice appended line")
.await
.expect("alice append failed");
// Shared document must be unchanged (write isolation)
let shared_doc = ws_shared
.read("notes/log.md")
.await
.expect("shared read failed");
assert_eq!(
shared_doc.content, "shared original content",
"Append must not modify the secondary scope's document"
);
// Alice should have her own copy with the appended content
let ws_alice_plain = Workspace::new(alice_id, pool.clone());
let alice_doc = ws_alice_plain
.read("notes/log.md")
.await
.expect("alice read failed");
assert_eq!(
alice_doc.content, "alice appended line",
"Append should create a new document in alice's scope"
);
cleanup_user(&pool, shared_id).await;
cleanup_user(&pool, alice_id).await;
}