mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-09-01 09:09:19 +00:00
feat(workspace): multi-scope workspace reads (#1117)
* feat(workspace): multi-scope workspace reads Adds the ability for a workspace to read from multiple user scopes while keeping writes isolated to the primary scope. Configuration via WORKSPACE_READ_SCOPES env var (comma-separated user IDs). Includes identity file isolation (read_primary), multi-scope search, list, and read operations, WorkspaceConfig refactor, and comprehensive integration tests. * fix: address review feedback for multi-scope workspace reads - fix(memory): deduplicate timezone parsing for daily_log target parse_timezone was called twice when target was "daily_log" without a layer — once in path resolution, again in the fallback. Now computed once and reused. - fix(config): add character validation for WORKSPACE_READ_SCOPES and layer scopes — both enforce [a-zA-Z0-9_-] to prevent path traversal or injection via scope strings used as user_id in SQL queries. - fix(config): use chars().take(32) instead of byte-index slicing for scope length error messages (UTF-8 safety). - fix(error): remove unused WorkspaceError::NotFound variant Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * style: downgrade search log to debug, add comments on list iteration - Downgrade hybrid_search_multi tracing::info! to debug! — fires on every multi-scope search with the default backend, too noisy for info - Add comments explaining why list/list_all iterate per-scope instead of using _multi trait methods (identity path filtering needs scope attribution that merged results lose) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: [email protected] <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
+167
-4
@@ -37,6 +37,25 @@ pub mod paths {
|
||||
pub const ASSISTANT_DIRECTIVES: &str = "context/assistant-directives.md";
|
||||
}
|
||||
|
||||
/// Paths treated as identity documents for multi-scope isolation.
|
||||
///
|
||||
/// These files are always read from the primary scope only — never from
|
||||
/// secondary read scopes. This prevents silent identity inheritance
|
||||
/// (e.g., user A accidentally presenting as user B).
|
||||
pub const IDENTITY_PATHS: &[&str] = &[
|
||||
paths::IDENTITY,
|
||||
paths::SOUL,
|
||||
paths::AGENTS,
|
||||
paths::USER,
|
||||
paths::TOOLS,
|
||||
paths::BOOTSTRAP,
|
||||
];
|
||||
|
||||
/// Check if a path is an identity document that must be isolated to primary scope.
|
||||
pub fn is_identity_path(path: &str) -> bool {
|
||||
IDENTITY_PATHS.contains(&path)
|
||||
}
|
||||
|
||||
/// A memory document stored in the database.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct MemoryDocument {
|
||||
@@ -101,10 +120,7 @@ impl MemoryDocument {
|
||||
|
||||
/// Check if this is a well-known identity document.
|
||||
pub fn is_identity_document(&self) -> bool {
|
||||
matches!(
|
||||
self.path.as_str(),
|
||||
paths::IDENTITY | paths::SOUL | paths::AGENTS | paths::USER
|
||||
)
|
||||
is_identity_path(&self.path)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -128,6 +144,42 @@ impl WorkspaceEntry {
|
||||
}
|
||||
}
|
||||
|
||||
/// Merge workspace entries from multiple scopes into a deduplicated, sorted list.
|
||||
///
|
||||
/// When the same path appears in multiple scopes:
|
||||
/// - Keeps the most recent `updated_at`
|
||||
/// - If any scope marks it as a directory, the merged entry is a directory
|
||||
pub fn merge_workspace_entries(
|
||||
entries: impl IntoIterator<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 {
|
||||
@@ -226,4 +278,115 @@ mod tests {
|
||||
};
|
||||
assert_eq!(entry.name(), "alpha");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_merge_workspace_entries_empty() {
|
||||
let result = merge_workspace_entries(vec![]);
|
||||
assert!(result.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_merge_workspace_entries_keeps_newer_timestamp_and_preview() {
|
||||
use chrono::TimeZone;
|
||||
let old_ts = chrono::Utc.with_ymd_and_hms(2025, 1, 1, 0, 0, 0).unwrap();
|
||||
let new_ts = chrono::Utc.with_ymd_and_hms(2025, 6, 1, 0, 0, 0).unwrap();
|
||||
|
||||
let entries = vec![
|
||||
WorkspaceEntry {
|
||||
path: "notes.md".to_string(),
|
||||
is_directory: false,
|
||||
updated_at: Some(old_ts),
|
||||
content_preview: Some("old".to_string()),
|
||||
},
|
||||
WorkspaceEntry {
|
||||
path: "notes.md".to_string(),
|
||||
is_directory: false,
|
||||
updated_at: Some(new_ts),
|
||||
content_preview: Some("new".to_string()),
|
||||
},
|
||||
];
|
||||
|
||||
let result = merge_workspace_entries(entries);
|
||||
assert_eq!(result.len(), 1);
|
||||
assert_eq!(result[0].updated_at, Some(new_ts));
|
||||
assert_eq!(result[0].content_preview, Some("new".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_merge_workspace_entries_directory_wins() {
|
||||
let entries = vec![
|
||||
WorkspaceEntry {
|
||||
path: "projects".to_string(),
|
||||
is_directory: false,
|
||||
updated_at: None,
|
||||
content_preview: Some("file content".to_string()),
|
||||
},
|
||||
WorkspaceEntry {
|
||||
path: "projects".to_string(),
|
||||
is_directory: true,
|
||||
updated_at: None,
|
||||
content_preview: None,
|
||||
},
|
||||
];
|
||||
|
||||
let result = merge_workspace_entries(entries);
|
||||
assert_eq!(result.len(), 1);
|
||||
assert!(result[0].is_directory);
|
||||
assert!(result[0].content_preview.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_merge_workspace_entries_fills_missing_timestamp() {
|
||||
use chrono::TimeZone;
|
||||
let ts = chrono::Utc.with_ymd_and_hms(2025, 3, 1, 0, 0, 0).unwrap();
|
||||
|
||||
let entries = vec![
|
||||
WorkspaceEntry {
|
||||
path: "a.md".to_string(),
|
||||
is_directory: false,
|
||||
updated_at: None,
|
||||
content_preview: None,
|
||||
},
|
||||
WorkspaceEntry {
|
||||
path: "a.md".to_string(),
|
||||
is_directory: false,
|
||||
updated_at: Some(ts),
|
||||
content_preview: None,
|
||||
},
|
||||
];
|
||||
|
||||
let result = merge_workspace_entries(entries);
|
||||
assert_eq!(result.len(), 1);
|
||||
assert_eq!(result[0].updated_at, Some(ts));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_merge_workspace_entries_sorted_by_path() {
|
||||
let entries = vec![
|
||||
WorkspaceEntry {
|
||||
path: "z.md".to_string(),
|
||||
is_directory: false,
|
||||
updated_at: None,
|
||||
content_preview: None,
|
||||
},
|
||||
WorkspaceEntry {
|
||||
path: "a.md".to_string(),
|
||||
is_directory: false,
|
||||
updated_at: None,
|
||||
content_preview: None,
|
||||
},
|
||||
WorkspaceEntry {
|
||||
path: "m.md".to_string(),
|
||||
is_directory: false,
|
||||
updated_at: None,
|
||||
content_preview: None,
|
||||
},
|
||||
];
|
||||
|
||||
let result = merge_workspace_entries(entries);
|
||||
assert_eq!(result.len(), 3);
|
||||
assert_eq!(result[0].path, "a.md");
|
||||
assert_eq!(result[1].path, "m.md");
|
||||
assert_eq!(result[2].path, "z.md");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user