From a5d207821d06689fd9a2863144449d210492caff Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 26 Jul 2026 16:18:07 +0900 Subject: [PATCH] memory: simplify document consolidation tools --- crates/memory/src/backend.rs | 361 +----------- crates/memory/src/lib.rs | 10 +- crates/memory/src/tool/delete.rs | 159 ------ crates/memory/src/tool/edit.rs | 391 ------------- crates/memory/src/tool/mod.rs | 98 ---- crates/memory/src/tool/query.rs | 517 ------------------ crates/memory/src/tool/read.rs | 327 ----------- crates/memory/src/tool/write.rs | 341 ------------ crates/worker/src/feature/builtin/memory.rs | 48 +- crates/worker/src/prompt/catalog.rs | 22 + crates/workspace-server/src/authority.rs | 2 +- crates/workspace-server/src/memory_backend.rs | 110 ++-- crates/workspace-server/src/store.rs | 2 +- .../internal/memory_consolidation_system.md | 27 +- 14 files changed, 183 insertions(+), 2232 deletions(-) delete mode 100644 crates/memory/src/tool/delete.rs delete mode 100644 crates/memory/src/tool/edit.rs delete mode 100644 crates/memory/src/tool/mod.rs delete mode 100644 crates/memory/src/tool/query.rs delete mode 100644 crates/memory/src/tool/read.rs delete mode 100644 crates/memory/src/tool/write.rs diff --git a/crates/memory/src/backend.rs b/crates/memory/src/backend.rs index d0a4dc14..df8c548b 100644 --- a/crates/memory/src/backend.rs +++ b/crates/memory/src/backend.rs @@ -8,7 +8,6 @@ use std::fs; use std::io; use std::io::Write; -use std::path::PathBuf; use chrono::Utc; use schemars::JsonSchema; @@ -21,7 +20,6 @@ use crate::extract::{ ExtractedCandidate, ExtractedPayload, StagingEvidence, write_staging, write_staging_candidate, }; use crate::schema::{SourceEvidenceRef, SourceRef}; -use crate::tool::MemoryToolKind; use crate::workspace::WorkspaceLayout; #[derive(Debug, Clone, Serialize, Deserialize)] @@ -30,10 +28,6 @@ pub enum MemoryBackendOperation { Query(MemoryQueryOperation), ReadDocument(MemoryDocumentReadOperation), UpdateDocument(MemoryDocumentUpdateOperation), - Read(MemoryReadOperation), - Write(MemoryWriteOperation), - Edit(MemoryEditOperation), - Delete(MemoryDeleteOperation), ResidentSummary(MemoryResidentSummaryOperation), AppendAudit(MemoryAppendAuditOperation), StageCandidate(MemoryStageCandidateOperation), @@ -85,46 +79,12 @@ pub struct MemoryDocumentReadOperation { #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] pub struct MemoryDocumentUpdateOperation { - pub body_md: String, -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct MemoryReadOperation { - pub kind: MemoryToolKind, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub slug: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub offset: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub limit: Option, -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct MemoryWriteOperation { - pub kind: MemoryToolKind, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub slug: Option, - pub content: String, -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct MemoryEditOperation { - pub kind: MemoryToolKind, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub slug: Option, pub old_string: String, pub new_string: String, #[serde(default)] pub replace_all: bool, } -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct MemoryDeleteOperation { - pub kind: MemoryToolKind, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub slug: Option, -} - #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct MemoryResidentSummaryOperation {} @@ -182,19 +142,13 @@ pub enum MemoryStagingCloseAction { #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] pub struct MemoryStagingAffectedMemory { - pub kind: MemoryToolKind, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub slug: Option, pub operation: MemoryStagingAffectedMemoryOperation, } #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum MemoryStagingAffectedMemoryOperation { - Read, - Write, Edit, - Delete, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -244,9 +198,9 @@ pub fn execute_memory_backend_operation( operation: MemoryBackendOperation, ) -> io::Result { match operation { - MemoryBackendOperation::Query(operation) => { - execute_query(layout, operation).map(MemoryBackendOperationResult::ToolOutput) - } + MemoryBackendOperation::Query(operation) => Ok(MemoryBackendOperationResult::ToolOutput( + execute_query(layout, operation), + )), MemoryBackendOperation::ReadDocument(_operation) => Err(io::Error::new( io::ErrorKind::Unsupported, "Memory document operations require WorkspaceAuthority-backed executor", @@ -255,18 +209,6 @@ pub fn execute_memory_backend_operation( io::ErrorKind::Unsupported, "Memory document operations require WorkspaceAuthority-backed executor", )), - MemoryBackendOperation::Read(operation) => { - execute_read(layout, operation).map(MemoryBackendOperationResult::ToolOutput) - } - MemoryBackendOperation::Write(operation) => { - execute_write(layout, operation).map(MemoryBackendOperationResult::ToolOutput) - } - MemoryBackendOperation::Edit(operation) => { - execute_edit(layout, operation).map(MemoryBackendOperationResult::ToolOutput) - } - MemoryBackendOperation::Delete(operation) => { - execute_delete(layout, operation).map(MemoryBackendOperationResult::ToolOutput) - } MemoryBackendOperation::ResidentSummary(_operation) => Ok( MemoryBackendOperationResult::ToolOutput(execute_resident_summary(layout)), ), @@ -328,185 +270,30 @@ fn execute_resident_summary(layout: &WorkspaceLayout) -> MemoryToolOutput { } } -fn execute_query( - layout: &WorkspaceLayout, - operation: MemoryQueryOperation, -) -> io::Result { - let mut entries = Vec::new(); - for kind in [ - MemoryToolKind::Summary, - MemoryToolKind::Decision, - MemoryToolKind::Request, - ] { - entries.extend(query_kind(layout, kind, operation.query.as_deref())?); - } - - if entries.is_empty() { +fn execute_query(layout: &WorkspaceLayout, operation: MemoryQueryOperation) -> MemoryToolOutput { + let Some(document) = crate::collect_resident_summary(layout) else { let suffix = operation .query .as_deref() .map(|query| format!(" matching {query:?}")) .unwrap_or_default(); - return Ok(MemoryToolOutput { - summary: format!("No memory records found{suffix}"), + return MemoryToolOutput { + summary: format!("No memory document content found{suffix}"), content: None, - }); - } - - let body = entries.join("\n\n"); - Ok(tool_output_from_string(body)) -} - -fn query_kind( - layout: &WorkspaceLayout, - kind: MemoryToolKind, - query: Option<&str>, -) -> io::Result> { - let mut entries = Vec::new(); - match kind { - MemoryToolKind::Summary => { - let path = memory_path(layout, kind, None)?; - if !path.exists() { - return Ok(entries); - } - let content = fs::read_to_string(&path)?; - if matches_query(&content, query) { - entries.push(format!("kind=summary\n{}", excerpt(&content, query))); - } - } - MemoryToolKind::Decision | MemoryToolKind::Request => { - let dir = record_dir(layout, kind); - if !dir.exists() { - return Ok(entries); - } - let mut files: Vec = fs::read_dir(&dir)? - .filter_map(Result::ok) - .map(|entry| entry.path()) - .filter(|path| path.extension().and_then(|ext| ext.to_str()) == Some("md")) - .collect(); - files.sort(); - for path in files { - let content = fs::read_to_string(&path)?; - if matches_query(&content, query) { - let slug = path - .file_stem() - .and_then(|value| value.to_str()) - .unwrap_or(""); - entries.push(format!( - "kind={} slug={}\n{}", - kind.as_str(), - slug, - excerpt(&content, query) - )); - } - } - } - } - Ok(entries) -} - -fn execute_read( - layout: &WorkspaceLayout, - operation: MemoryReadOperation, -) -> io::Result { - let path = memory_path(layout, operation.kind, operation.slug.as_deref())?; - let content = fs::read_to_string(&path)?; - let offset = operation.offset.unwrap_or(0); - let limit = operation.limit.unwrap_or(2000); - let lines: Vec<&str> = content.lines().collect(); - let selected = lines - .iter() - .enumerate() - .skip(offset) - .take(limit) - .map(|(index, line)| format!("{:>6}\t{}", index + 1, line)) - .collect::>() - .join("\n"); - let total = lines.len(); - Ok(MemoryToolOutput { - summary: format!( - "Read {} lines from {}{}", - selected.lines().count(), - operation.kind.as_str(), - operation - .slug - .as_deref() - .map(|slug| format!("/{slug}")) - .unwrap_or_default() - ), - content: Some(format!( - "total_lines={total} offset={offset} limit={limit}\n{selected}" - )), - }) -} - -fn execute_write( - layout: &WorkspaceLayout, - operation: MemoryWriteOperation, -) -> io::Result { - let path = memory_path(layout, operation.kind, operation.slug.as_deref())?; - validate_slug_rules(operation.kind, operation.slug.as_deref())?; - validate_memory_content( - operation.kind, - operation.slug.as_deref(), - &operation.content, - )?; - if let Some(parent) = path.parent() { - fs::create_dir_all(parent)?; - } - fs::write(&path, operation.content)?; - Ok(MemoryToolOutput { - summary: format!("Wrote {} record", operation.kind.as_str()), - content: Some(path.display().to_string()), - }) -} - -fn execute_edit( - layout: &WorkspaceLayout, - operation: MemoryEditOperation, -) -> io::Result { - validate_slug_rules(operation.kind, operation.slug.as_deref())?; - if operation.old_string == operation.new_string { - return Err(invalid_input("old_string and new_string must differ")); - } - let path = memory_path(layout, operation.kind, operation.slug.as_deref())?; - let original = fs::read_to_string(&path)?; - let count = original.matches(&operation.old_string).count(); - if count == 0 { - return Err(invalid_input("old_string was not found")); - } - if count > 1 && !operation.replace_all { - return Err(invalid_input( - "old_string matched more than once; set replace_all=true to replace every occurrence", - )); - } - let updated = if operation.replace_all { - original.replace(&operation.old_string, &operation.new_string) - } else { - original.replacen(&operation.old_string, &operation.new_string, 1) + }; }; - validate_memory_content(operation.kind, operation.slug.as_deref(), &updated)?; - fs::write(&path, updated)?; - Ok(MemoryToolOutput { - summary: format!( - "Edited {} occurrence(s)", - if operation.replace_all { count } else { 1 } - ), - content: Some(path.display().to_string()), - }) -} - -fn execute_delete( - layout: &WorkspaceLayout, - operation: MemoryDeleteOperation, -) -> io::Result { - validate_slug_rules(operation.kind, operation.slug.as_deref())?; - let path = memory_path(layout, operation.kind, operation.slug.as_deref())?; - fs::remove_file(&path)?; - Ok(MemoryToolOutput { - summary: format!("Deleted {} record", operation.kind.as_str()), - content: Some(path.display().to_string()), - }) + if !matches_query(&document, operation.query.as_deref()) { + let suffix = operation + .query + .as_deref() + .map(|query| format!(" matching {query:?}")) + .unwrap_or_default(); + return MemoryToolOutput { + summary: format!("No memory document content found{suffix}"), + content: None, + }; + } + tool_output_from_string(excerpt(&document, operation.query.as_deref())) } fn execute_staging_list( @@ -563,7 +350,6 @@ fn execute_staging_close( if operation.reason.trim().is_empty() { return Err(invalid_input("reason is required")); } - validate_affected_memory(&operation.affected_memory)?; let entry = find_staging_entry(layout, &operation.candidate_id)?; let disposition = MemoryStagingCloseDispositionRecord { schema_version: 1, @@ -605,107 +391,6 @@ fn find_staging_entry( .ok_or_else(|| invalid_input(format!("staging candidate not found: {candidate_id}"))) } -fn validate_affected_memory(records: &[MemoryStagingAffectedMemory]) -> io::Result<()> { - for record in records { - validate_slug_rules(record.kind, record.slug.as_deref())?; - } - Ok(()) -} - -fn memory_path( - layout: &WorkspaceLayout, - kind: MemoryToolKind, - slug: Option<&str>, -) -> io::Result { - match kind { - MemoryToolKind::Summary => { - if slug.is_some() { - return Err(invalid_input("summary records do not accept slug")); - } - Ok(layout.memory_dir().join("summary.md")) - } - MemoryToolKind::Decision => { - Ok(record_dir(layout, kind).join(format!("{}.md", required_slug(slug)?))) - } - MemoryToolKind::Request => { - Ok(record_dir(layout, kind).join(format!("{}.md", required_slug(slug)?))) - } - } -} - -fn record_dir(layout: &WorkspaceLayout, kind: MemoryToolKind) -> PathBuf { - match kind { - MemoryToolKind::Summary => layout.memory_dir(), - MemoryToolKind::Decision => layout.memory_dir().join("decisions"), - MemoryToolKind::Request => layout.memory_dir().join("requests"), - } -} - -fn required_slug(slug: Option<&str>) -> io::Result<&str> { - slug.filter(|value| !value.trim().is_empty()) - .ok_or_else(|| invalid_input("slug is required for this memory kind")) -} - -fn validate_slug_rules(kind: MemoryToolKind, slug: Option<&str>) -> io::Result<()> { - match kind { - MemoryToolKind::Summary => { - if slug.is_some() { - return Err(invalid_input("summary records do not accept slug")); - } - } - MemoryToolKind::Decision | MemoryToolKind::Request => { - validate_slug(required_slug(slug)?)?; - } - } - Ok(()) -} - -fn validate_slug(slug: &str) -> io::Result<()> { - if slug.is_empty() - || slug.contains('/') - || slug.contains('\\') - || slug == "." - || slug == ".." - || slug.starts_with('.') - || !slug - .chars() - .all(|ch| ch.is_ascii_lowercase() || ch.is_ascii_digit() || ch == '-') - { - return Err(invalid_input( - "slug must use lowercase ASCII letters, digits, and '-' only", - )); - } - Ok(()) -} - -fn validate_memory_content( - kind: MemoryToolKind, - slug: Option<&str>, - content: &str, -) -> io::Result<()> { - if content.trim().is_empty() { - return Err(invalid_input("content must not be empty")); - } - match kind { - MemoryToolKind::Summary => { - if !content.trim_start().starts_with("# Memory summary") { - return Err(invalid_input( - "summary content must start with '# Memory summary'", - )); - } - } - MemoryToolKind::Decision | MemoryToolKind::Request => { - validate_slug_rules(kind, slug)?; - if !content.trim_start().starts_with("---") { - return Err(invalid_input( - "memory record content must start with YAML frontmatter", - )); - } - } - } - Ok(()) -} - fn matches_query(content: &str, query: Option<&str>) -> bool { match query.map(str::trim).filter(|query| !query.is_empty()) { Some(query) => content.to_lowercase().contains(&query.to_lowercase()), @@ -822,10 +507,8 @@ mod tests { MemoryBackendOperation::StagingClose(MemoryStagingCloseOperation { candidate_id: candidate_id.clone(), action: MemoryStagingCloseAction::Applied, - reason: "Merged into durable request memory.".into(), + reason: "Merged into the Memory document.".into(), affected_memory: vec![MemoryStagingAffectedMemory { - kind: MemoryToolKind::Request, - slug: Some("review-preferences".into()), operation: MemoryStagingAffectedMemoryOperation::Edit, }], }), @@ -837,6 +520,6 @@ mod tests { let resolutions = fs::read_to_string(layout.memory_dir().join(STAGING_RESOLUTIONS_FILE)).unwrap(); assert!(resolutions.contains(&candidate_id)); - assert!(resolutions.contains("Merged into durable request memory.")); + assert!(resolutions.contains("Merged into the Memory document.")); } } diff --git a/crates/memory/src/lib.rs b/crates/memory/src/lib.rs index 77447380..44dac013 100644 --- a/crates/memory/src/lib.rs +++ b/crates/memory/src/lib.rs @@ -1,9 +1,8 @@ -//! Memory subsystem: persistence layer for `memory/*` records. +//! Memory subsystem support shared by Worker and Workspace authority paths. //! -//! Self-contained: provides its own Tool implementations (read/write/edit) -//! that target `/memory/` only, with a pre-write Linter built in. -//! Generic CRUD tools (in the `tools` crate) must not touch this directory — -//! Worker is responsible for denying it at the Scope level when memory is enabled. +//! Normal runtime Memory access is mediated through Workspace-backed backend +//! operations. Generic filesystem tools must not touch local memory paths; Worker +//! is responsible for denying them at the Scope level when memory is enabled. pub mod audit; pub mod backend; @@ -14,7 +13,6 @@ pub mod linter; pub mod resident; pub mod schema; pub mod scope; -pub mod tool; pub mod usage; pub mod workspace; diff --git a/crates/memory/src/tool/delete.rs b/crates/memory/src/tool/delete.rs deleted file mode 100644 index 4263c4f0..00000000 --- a/crates/memory/src/tool/delete.rs +++ /dev/null @@ -1,159 +0,0 @@ -//! `MemoryDelete` tool for removing memory records with audit logging. - -use std::sync::Arc; - -use async_trait::async_trait; -use llm_engine::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput}; -use serde::Deserialize; - -use crate::audit::{AuditStatus, RecordOperationAudit, append_record_operation, file_hash}; -use crate::tool::MemoryToolKind; -use crate::workspace::WorkspaceLayout; - -const DESCRIPTION: &str = "Delete an existing memory record selected by `kind` + `slug`. \ -For `summary` omit `slug`; for the others `slug` is required. The delete is audited and cannot target \ -workflow or staging/log files."; - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct DeleteParams { - /// Kind of record to delete. - kind: MemoryToolKind, - /// Slug. Required for everything except `summary`; forbidden for `summary`. - #[serde(default)] - slug: Option, -} - -struct MemoryDeleteTool { - layout: WorkspaceLayout, -} - -#[async_trait] -impl Tool for MemoryDeleteTool { - async fn execute( - &self, - input_json: &str, - _ctx: llm_engine::tool::ToolExecutionContext, - ) -> Result { - let params: DeleteParams = serde_json::from_str(input_json) - .map_err(|e| ToolError::InvalidArgument(format!("invalid MemoryDelete input: {e}")))?; - let path = params - .kind - .resolve_path(&self.layout, params.slug.as_deref())?; - let kind = params.kind.to_string(); - let slug = audit_slug(¶ms.kind, params.slug.as_deref()); - let before_hash = file_hash(&path).ok().flatten(); - if before_hash.is_none() { - let reason = format!("record not found: {}", path.display()); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "delete".to_string(), - status: AuditStatus::Failed, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::ExecutionFailed(reason)); - } - - if let Err(err) = std::fs::remove_file(&path) { - let reason = format!("failed to delete {}: {err}", path.display()); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "delete".to_string(), - status: AuditStatus::Failed, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::ExecutionFailed(reason)); - } - - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "delete".to_string(), - status: AuditStatus::Success, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash: None, - reason: None, - }, - ); - - Ok(ToolOutput { - summary: format!("Deleted {}", path.display()), - content: None, - }) - } -} - -pub fn delete_tool(layout: WorkspaceLayout) -> ToolDefinition { - Arc::new(move || { - let schema = schemars::schema_for!(DeleteParams); - let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); - let meta = ToolMeta::new("MemoryDelete") - .description(DESCRIPTION) - .input_schema(schema_value); - let tool: Arc = Arc::new(MemoryDeleteTool { - layout: layout.clone(), - }); - (meta, tool) - }) -} - -fn audit_slug(kind: &MemoryToolKind, slug: Option<&str>) -> String { - match kind { - MemoryToolKind::Summary => "summary".to_string(), - _ => slug.unwrap_or("").to_string(), - } -} - -#[cfg(test)] -mod tests { - use super::*; - use chrono::Utc; - use tempfile::TempDir; - - #[tokio::test] - async fn delete_removes_file_and_audits() { - let dir = TempDir::new().unwrap(); - let layout = WorkspaceLayout::new(dir.path().to_path_buf()); - std::fs::create_dir_all(layout.decisions_dir()).unwrap(); - let path = layout.decisions_dir().join("obsolete.md"); - let now = Utc::now().to_rfc3339(); - std::fs::write( - &path, - format!( - "---\ncreated_at: {now}\nupdated_at: {now}\nsources: []\nstatus: open\n---\nold" - ), - ) - .unwrap(); - - let (_, tool) = delete_tool(layout.clone())(); - let out = tool - .execute( - r#"{"kind":"decision","slug":"obsolete"}"#, - Default::default(), - ) - .await - .unwrap(); - assert!(out.summary.contains("Deleted")); - assert!(!path.exists()); - let log = std::fs::read_to_string(layout.audit_current_log_path()).unwrap(); - assert!(log.contains(r#""event":"record_operation""#)); - assert!(log.contains(r#""op":"delete""#)); - assert!(log.contains(r#""status":"success""#)); - } -} diff --git a/crates/memory/src/tool/edit.rs b/crates/memory/src/tool/edit.rs deleted file mode 100644 index 1b24824c..00000000 --- a/crates/memory/src/tool/edit.rs +++ /dev/null @@ -1,391 +0,0 @@ -//! `MemoryEdit` tool — partial string replacement on an existing memory record. -//! -//! Reads current content by `(kind, slug)`, applies the replacement, -//! runs the Linter on the result, writes only on success. The -//! current-then-write window is single-tool-call narrow; an external -//! tracker is intentionally omitted (memory tools are self-contained, -//! no `tools` crate dep). - -use std::sync::Arc; - -use async_trait::async_trait; -use llm_engine::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput}; -use serde::Deserialize; - -use crate::audit::{ - AuditStatus, RecordOperationAudit, append_record_operation, file_hash, hash_bytes, -}; -use crate::linter::{LintReport, Linter, WriteMode}; -use crate::tool::MemoryToolKind; -use crate::workspace::WorkspaceLayout; - -const DESCRIPTION: &str = "Replace a substring in an existing memory \ -record selected by `kind` + `slug`. By default `old_string` must be unique in the \ -file; set `replace_all: true` to replace every occurrence. The resulting content \ -is re-validated by the memory linter; failure leaves the file untouched."; - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct EditParams { - /// Record kind: `summary` | `decision` | `request`. - kind: MemoryToolKind, - /// Slug. Required for everything except `summary`; forbidden for `summary`. - #[serde(default)] - slug: Option, - /// String to replace. Must be unique in the file unless `replace_all` is true. - old_string: String, - /// Replacement string. Must differ from `old_string`. - new_string: String, - /// Replace all occurrences. Defaults to false. - #[serde(default)] - replace_all: bool, -} - -struct EditTool { - layout: WorkspaceLayout, - linter: Linter, -} - -#[async_trait] -impl Tool for EditTool { - async fn execute( - &self, - input_json: &str, - _ctx: llm_engine::tool::ToolExecutionContext, - ) -> Result { - let params: EditParams = serde_json::from_str(input_json) - .map_err(|e| ToolError::InvalidArgument(format!("invalid MemoryEdit input: {e}")))?; - - if params.old_string.is_empty() { - return Err(ToolError::InvalidArgument( - "old_string must not be empty".into(), - )); - } - if params.old_string == params.new_string { - return Err(ToolError::InvalidArgument( - "old_string and new_string are identical".into(), - )); - } - - let path = params - .kind - .resolve_path(&self.layout, params.slug.as_deref())?; - let kind = params.kind.to_string(); - let slug = audit_slug(¶ms.kind, params.slug.as_deref()); - - let current_bytes = match std::fs::read(&path) { - Ok(bytes) => bytes, - Err(e) => { - let reason = match e.kind() { - std::io::ErrorKind::NotFound => format!( - "record not found (use MemoryWrite to create): {}", - path.display() - ), - _ => format!("read failed at {}: {e}", path.display()), - }; - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "edit".to_string(), - status: AuditStatus::Failed, - kind, - slug, - path: path.display().to_string(), - before_hash: None, - after_hash: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::ExecutionFailed(reason)); - } - }; - let before_hash = Some(hash_bytes(¤t_bytes)); - let current_text = match std::str::from_utf8(¤t_bytes) { - Ok(text) => text, - Err(_) => { - let reason = format!("file is not valid UTF-8: {}", path.display()); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "edit".to_string(), - status: AuditStatus::Failed, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::InvalidArgument(reason)); - } - }; - - let count = current_text.matches(¶ms.old_string).count(); - if count == 0 { - let reason = format!("old_string not found in {}", path.display()); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "edit".to_string(), - status: AuditStatus::Failed, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::InvalidArgument(reason)); - } - if !params.replace_all && count > 1 { - let reason = format!( - "old_string occurs {count} times in {}; pass replace_all: true or narrow the snippet", - path.display() - ); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "edit".to_string(), - status: AuditStatus::Failed, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::InvalidArgument(reason)); - } - - let new_text = if params.replace_all { - current_text.replace(¶ms.old_string, ¶ms.new_string) - } else { - current_text.replacen(¶ms.old_string, ¶ms.new_string, 1) - }; - let occurrences = if params.replace_all { count } else { 1 }; - - let report = self.linter.lint(&path, &new_text, WriteMode::Update); - if report.has_errors() { - let reason = format_report(&report); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "edit".to_string(), - status: AuditStatus::Failed, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::InvalidArgument(reason)); - } - - if let Err(e) = std::fs::write(&path, new_text.as_bytes()) { - let reason = format!("failed to write {}: {e}", path.display()); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "edit".to_string(), - status: AuditStatus::Failed, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::ExecutionFailed(reason)); - } - let after_hash = file_hash(&path).ok().flatten(); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "edit".to_string(), - status: AuditStatus::Success, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash, - reason: if report.warnings.is_empty() { - None - } else { - Some(format!("{} warning(s)", report.warnings.len())) - }, - }, - ); - - let summary = format!( - "Edited {} ({} replacement{}){}", - path.display(), - occurrences, - if occurrences == 1 { "" } else { "s" }, - warning_tail(&report), - ); - Ok(ToolOutput { - summary, - content: None, - }) - } -} - -fn audit_slug(kind: &MemoryToolKind, slug: Option<&str>) -> String { - match kind { - MemoryToolKind::Summary => "summary".to_string(), - _ => slug.unwrap_or("").to_string(), - } -} - -fn format_report(report: &LintReport) -> String { - use std::fmt::Write as _; - let mut buf = String::from("memory linter rejected the edit:"); - for e in &report.errors { - let _ = write!(&mut buf, "\n - {e}"); - } - if !report.warnings.is_empty() { - let _ = write!(&mut buf, "\nwarnings (informational):"); - for w in &report.warnings { - let _ = write!(&mut buf, "\n - {w}"); - } - } - buf -} - -fn warning_tail(report: &LintReport) -> String { - if report.warnings.is_empty() { - return String::new(); - } - let mut s = format!(" [{} warning(s)]", report.warnings.len()); - for w in &report.warnings { - use std::fmt::Write as _; - let _ = write!(&mut s, " {w};"); - } - s -} - -pub fn edit_tool(layout: WorkspaceLayout) -> ToolDefinition { - Arc::new(move || { - let schema = schemars::schema_for!(EditParams); - let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); - let meta = ToolMeta::new("MemoryEdit") - .description(DESCRIPTION) - .input_schema(schema_value); - let tool: Arc = Arc::new(EditTool { - layout: layout.clone(), - linter: Linter::new(layout.clone()), - }); - (meta, tool) - }) -} - -#[cfg(test)] -mod tests { - use super::*; - use chrono::Utc; - use std::path::PathBuf; - use tempfile::TempDir; - - fn now() -> String { - Utc::now().to_rfc3339() - } - - fn setup() -> (TempDir, WorkspaceLayout, PathBuf) { - let dir = TempDir::new().unwrap(); - let layout = WorkspaceLayout::new(dir.path().to_path_buf()); - let path = dir.path().join(".yoi/memory/decisions/foo.md"); - std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - let initial = format!( - "---\ncreated_at: {n}\nupdated_at: {n}\nsources: []\nstatus: open\n---\nbody body\n", - n = now() - ); - std::fs::write(&path, &initial).unwrap(); - (dir, layout, path) - } - - #[tokio::test] - async fn edit_simple_replace() { - let (_dir, layout, path) = setup(); - let (meta, tool) = edit_tool(layout)(); - assert_eq!(meta.name, "MemoryEdit"); - - let inp = serde_json::json!({ - "kind": "decision", - "slug": "foo", - "old_string": "body body", - "new_string": "edited", - }); - let out = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap(); - assert!(out.summary.contains("1 replacement")); - let after = std::fs::read_to_string(&path).unwrap(); - assert!(after.contains("edited")); - assert!(!after.contains("body body")); - } - - #[tokio::test] - async fn edit_resulting_invalid_frontmatter_rolled_back() { - let (_dir, layout, path) = setup(); - let (_, tool) = edit_tool(layout)(); - - // Drop the `status` field by replacing it with nothing. - let inp = serde_json::json!({ - "kind": "decision", - "slug": "foo", - "old_string": "status: open\n", - "new_string": "", - }); - let err = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap_err(); - let msg = format!("{err}"); - assert!(msg.contains("status") || msg.contains("missing")); - - // File untouched. - let after = std::fs::read_to_string(&path).unwrap(); - assert!(after.contains("status: open")); - } - - #[tokio::test] - async fn edit_missing_record() { - let (_dir, layout, _) = setup(); - let (_, tool) = edit_tool(layout)(); - let inp = serde_json::json!({ - "kind": "decision", - "slug": "ghost", - "old_string": "x", - "new_string": "y", - }); - let err = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap_err(); - assert!(matches!(err, ToolError::ExecutionFailed(_))); - } - - #[tokio::test] - async fn edit_workflow_kind_rejected() { - // Workflow is not exposed via MemoryToolKind, so deserialization fails. - let (_dir, layout, _) = setup(); - let (_, tool) = edit_tool(layout)(); - let inp = serde_json::json!({ - "kind": "workflow", - "slug": "wf", - "old_string": "x", - "new_string": "y", - }); - let err = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap_err(); - assert!(matches!(err, ToolError::InvalidArgument(_))); - } -} diff --git a/crates/memory/src/tool/mod.rs b/crates/memory/src/tool/mod.rs deleted file mode 100644 index a9767547..00000000 --- a/crates/memory/src/tool/mod.rs +++ /dev/null @@ -1,98 +0,0 @@ -//! Memory-scoped tools: Read / Write / Edit / Search. -//! -//! All four take `kind` + `slug` (Summary takes only `kind`) and -//! resolve the path through [`WorkspaceLayout`]. The agent never has -//! to know the on-disk layout — Search returns `{slug, kind, ...}` and -//! that pair feeds straight into Read / Edit. - -mod delete; -mod edit; -mod query; -mod read; -mod write; - -pub use delete::delete_tool; - -use std::path::PathBuf; - -use llm_engine::tool::ToolError; -use serde::Deserialize; - -use crate::Slug; -use crate::workspace::{RecordKind, WorkspaceLayout}; - -pub use edit::edit_tool; -pub use query::{QueryConfig, memory_query_tool}; -pub use read::{read_tool, read_tool_with_usage}; -pub use write::write_tool; - -/// Kinds the memory tools accept as input. `Workflow` is intentionally -/// excluded — workflows are sub-Engine context, not agent-editable. -#[derive( - Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, Deserialize, schemars::JsonSchema, -)] -#[serde(rename_all = "lowercase")] -pub enum MemoryToolKind { - Summary, - Decision, - Request, -} - -impl std::fmt::Display for MemoryToolKind { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(match self { - Self::Summary => "summary", - Self::Decision => "decision", - Self::Request => "request", - }) - } -} - -impl MemoryToolKind { - pub fn as_str(self) -> &'static str { - match self { - Self::Summary => "summary", - Self::Decision => "decision", - Self::Request => "request", - } - } - - pub fn record_kind(self) -> RecordKind { - match self { - Self::Summary => RecordKind::Summary, - Self::Decision => RecordKind::Decision, - Self::Request => RecordKind::Request, - } - } - - /// Resolve `(kind, slug)` to an absolute path under the workspace. - /// Summary forbids a slug; the per-record kinds require one. - pub fn resolve_path( - self, - layout: &WorkspaceLayout, - slug: Option<&str>, - ) -> Result { - match self { - Self::Summary => { - if slug.is_some() { - return Err(ToolError::InvalidArgument( - "kind=summary does not accept a slug".to_string(), - )); - } - Ok(layout.summary_path()) - } - other => { - let raw = slug.ok_or_else(|| { - ToolError::InvalidArgument(format!("kind={} requires `slug`", other.as_str())) - })?; - let parsed = - Slug::parse(raw).map_err(|e| ToolError::InvalidArgument(e.to_string()))?; - Ok(match other { - Self::Decision => layout.decision_path(&parsed), - Self::Request => layout.request_path(&parsed), - Self::Summary => unreachable!(), - }) - } - } - } -} diff --git a/crates/memory/src/tool/query.rs b/crates/memory/src/tool/query.rs deleted file mode 100644 index 6fff7de2..00000000 --- a/crates/memory/src/tool/query.rs +++ /dev/null @@ -1,517 +0,0 @@ -//! `MemoryQuery` tool. -//! -//! Performs a case-insensitive substring scan over markdown record -//! files. With a `query` set, returns `{slug, kind, ..., excerpt}` hits -//! with `excerpt_lines` lines of context around each match. With `query` -//! omitted, returns one entry per file (no excerpt) so the agent can -//! enumerate what records exist without knowing what's inside them. -//! -//! - `MemoryQuery` walks `.yoi/memory/{summary.md,decisions/, -//! requests/}`. `.yoi/memory/_staging/`, -//! `.yoi/memory/_usage/`, and `.yoi/memory/_logs/` are excluded -//! by construction. -//! -//! No derived index — the file tree is the source of truth and is -//! re-scanned per call. 出現順: within a file by line order, across -//! files by sorted filename. - -use std::path::{Path, PathBuf}; -use std::sync::Arc; - -use async_trait::async_trait; -use llm_engine::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput}; -use serde::{Deserialize, Serialize}; - -use crate::audit::{AuditStatus, RecordUsageAudit, append_record_usage}; -use crate::workspace::WorkspaceLayout; - -const DEFAULT_RESULT_LIMIT: usize = 20; -const DEFAULT_EXCERPT_LINES: usize = 3; - -const MEMORY_QUERY_DESCRIPTION: &str = "Inspect memory records (summary / decisions / \ -requests). With `query` set, returns substring hits as `{slug, kind, excerpt}` entries \ -with line context. Omit `query` to list every record (one entry per file, no excerpt) \ -when you don't yet know what's in there. Result count is capped (configurable via the \ -manifest's `[memory]` section). Use the returned `slug` + `kind` with MemoryRead to fetch \ -the full record. Workflow and staging directories are not visible."; - -/// Tunables passed in from the manifest. -#[derive(Debug, Clone, Copy)] -pub struct QueryConfig { - pub result_limit: usize, - /// Lines of context before and after each matched line. Ignored - /// when the request omits `query`. - pub excerpt_lines: usize, -} - -impl Default for QueryConfig { - fn default() -> Self { - Self { - result_limit: DEFAULT_RESULT_LIMIT, - excerpt_lines: DEFAULT_EXCERPT_LINES, - } - } -} - -impl From<&manifest::MemoryConfig> for QueryConfig { - fn from(cfg: &manifest::MemoryConfig) -> Self { - let mut out = Self::default(); - if let Some(n) = cfg.query_result_limit { - out.result_limit = n; - } - if let Some(n) = cfg.query_excerpt_lines { - out.excerpt_lines = n; - } - out - } -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct MemoryQueryParams { - /// Optional substring filter. Case-insensitive. Omit to list every - /// record under the query scope. - #[serde(default)] - query: Option, -} - -#[derive(Debug, Serialize)] -struct MemoryRecord { - slug: String, - kind: &'static str, - #[serde(skip_serializing_if = "Option::is_none")] - excerpt: Option, -} - -struct MemoryQueryTool { - layout: WorkspaceLayout, - config: QueryConfig, -} - -#[async_trait] -impl Tool for MemoryQueryTool { - async fn execute( - &self, - input_json: &str, - _ctx: llm_engine::tool::ToolExecutionContext, - ) -> Result { - let params: MemoryQueryParams = serde_json::from_str(input_json) - .map_err(|e| ToolError::InvalidArgument(format!("invalid MemoryQuery input: {e}")))?; - let needle = match params.query.as_deref() { - Some(q) => match validate_query(q) { - Ok(q) => Some(q), - Err(err) => { - let _ = append_record_usage( - &self.layout, - RecordUsageAudit { - op: "query".to_string(), - status: AuditStatus::Failed, - kind: "memory".to_string(), - slug: None, - path: None, - query: params.query.clone(), - result_count: None, - reason: Some(err.to_string()), - }, - ); - return Err(err); - } - }, - None => None, - }; - - let mut records: Vec = Vec::new(); - let limit = self.config.result_limit; - let ctx = self.config.excerpt_lines; - - // summary - if records.len() < limit { - let summary_path = self.layout.summary_path(); - if summary_path.is_file() { - collect_memory_records( - &summary_path, - "summary", - "summary", - needle.as_deref(), - ctx, - limit - records.len(), - &mut records, - ); - } - } - - // decisions - if records.len() < limit { - for (path, slug) in list_md_files(&self.layout.decisions_dir()) { - if records.len() >= limit { - break; - } - collect_memory_records( - &path, - &slug, - "decision", - needle.as_deref(), - ctx, - limit - records.len(), - &mut records, - ); - } - } - - // requests - if records.len() < limit { - for (path, slug) in list_md_files(&self.layout.requests_dir()) { - if records.len() >= limit { - break; - } - collect_memory_records( - &path, - &slug, - "request", - needle.as_deref(), - ctx, - limit - records.len(), - &mut records, - ); - } - } - - let body = serde_json::to_string_pretty(&records) - .map_err(|e| ToolError::ExecutionFailed(format!("serialize records: {e}")))?; - let summary = match params.query.as_deref() { - Some(q) => format!("{} hit(s) for {q:?}", records.len()), - None => format!("{} record(s)", records.len()), - }; - let _ = append_record_usage( - &self.layout, - RecordUsageAudit { - op: "query".to_string(), - status: AuditStatus::Success, - kind: "memory".to_string(), - slug: None, - path: None, - query: params.query.clone(), - result_count: Some(records.len()), - reason: if records.len() >= limit { - Some("result_limit_reached".to_string()) - } else { - None - }, - }, - ); - Ok(ToolOutput { - summary, - content: Some(body), - }) - } -} - -fn collect_memory_records( - path: &Path, - slug: &str, - kind: &'static str, - needle_lower: Option<&str>, - ctx: usize, - remaining: usize, - out: &mut Vec, -) { - if remaining == 0 { - return; - } - match needle_lower { - Some(n) => { - scan_file(path, n, ctx, remaining, |excerpt| { - out.push(MemoryRecord { - slug: slug.to_string(), - kind, - excerpt: Some(excerpt), - }); - }); - } - None => { - out.push(MemoryRecord { - slug: slug.to_string(), - kind, - excerpt: None, - }); - } - } -} - -fn validate_query(query: &str) -> Result { - if query.trim().is_empty() { - return Err(ToolError::InvalidArgument( - "query must not be empty when provided; omit it to list all records".into(), - )); - } - Ok(query.to_lowercase()) -} - -/// Sorted list of `(path, slug)` for `*.md` files directly under `dir`. -/// Returns empty if the directory doesn't exist. -fn list_md_files(dir: &Path) -> Vec<(PathBuf, String)> { - let mut out: Vec<(PathBuf, String)> = Vec::new(); - let entries = match std::fs::read_dir(dir) { - Ok(it) => it, - Err(_) => return out, - }; - for entry in entries.flatten() { - let path = entry.path(); - if !path.is_file() { - continue; - } - let name = match path.file_name().and_then(|n| n.to_str()) { - Some(n) => n, - None => continue, - }; - let slug = match name.strip_suffix(".md") { - Some(s) => s.to_string(), - None => continue, - }; - out.push((path, slug)); - } - out.sort_by(|a, b| a.1.cmp(&b.1)); - out -} - -fn scan_file( - path: &Path, - needle_lower: &str, - ctx: usize, - remaining: usize, - mut on_match: impl FnMut(String), -) { - if remaining == 0 { - return; - } - let text = match std::fs::read_to_string(path) { - Ok(t) => t, - Err(_) => return, - }; - scan_text(&text, needle_lower, ctx, remaining, |e| on_match(e)); -} - -fn scan_text( - text: &str, - needle_lower: &str, - ctx: usize, - remaining: usize, - mut on_match: impl FnMut(String), -) { - if remaining == 0 { - return; - } - let lines: Vec<&str> = text.lines().collect(); - let mut produced = 0; - for (i, line) in lines.iter().enumerate() { - if produced >= remaining { - break; - } - if line.to_lowercase().contains(needle_lower) { - let start = i.saturating_sub(ctx); - let end = i.saturating_add(ctx + 1).min(lines.len()); - let excerpt = lines[start..end].join("\n"); - on_match(excerpt); - produced += 1; - } - } -} - -pub fn memory_query_tool(layout: WorkspaceLayout, config: QueryConfig) -> ToolDefinition { - Arc::new(move || { - let schema = schemars::schema_for!(MemoryQueryParams); - let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); - let meta = ToolMeta::new("MemoryQuery") - .description(MEMORY_QUERY_DESCRIPTION) - .input_schema(schema_value); - let tool: Arc = Arc::new(MemoryQueryTool { - layout: layout.clone(), - config, - }); - (meta, tool) - }) -} - -#[cfg(test)] -mod tests { - use super::*; - use chrono::Utc; - use tempfile::TempDir; - - fn now() -> String { - Utc::now().to_rfc3339() - } - - fn setup() -> (TempDir, WorkspaceLayout) { - let dir = TempDir::new().unwrap(); - let layout = WorkspaceLayout::new(dir.path().to_path_buf()); - std::fs::create_dir_all(dir.path().join(".yoi/memory/decisions")).unwrap(); - std::fs::create_dir_all(dir.path().join(".yoi/memory/requests")).unwrap(); - std::fs::create_dir_all(dir.path().join(".yoi/memory/_staging")).unwrap(); - (dir, layout) - } - - fn write_decision(dir: &Path, slug: &str, body: &str) { - let path = dir.join(".yoi/memory/decisions").join(format!("{slug}.md")); - let content = format!( - "---\ncreated_at: {n}\nupdated_at: {n}\nsources: []\nstatus: open\n---\n{body}", - n = now() - ); - std::fs::write(path, content).unwrap(); - } - - #[derive(Deserialize)] - struct OwnedMemoryRecord { - slug: String, - kind: String, - #[serde(default)] - excerpt: Option, - } - - fn parse_records(out: &ToolOutput) -> Vec { - let text = out.content.as_ref().unwrap_or(&out.summary); - serde_json::from_str(text).unwrap() - } - #[tokio::test] - async fn memory_query_finds_decision_body() { - let (dir, layout) = setup(); - write_decision(dir.path(), "alpha", "we chose Ollama because it works\n"); - write_decision(dir.path(), "beta", "no match here\n"); - let (_, tool) = memory_query_tool(layout, QueryConfig::default())(); - let inp = serde_json::json!({ "query": "ollama" }); - let out = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap(); - let records: Vec = parse_records(&out); - assert_eq!(records.len(), 1); - assert_eq!(records[0].slug, "alpha"); - assert_eq!(records[0].kind, "decision"); - assert!( - records[0] - .excerpt - .as_deref() - .unwrap() - .to_lowercase() - .contains("ollama") - ); - } - - #[tokio::test] - async fn memory_query_without_query_lists_all_records() { - let (dir, layout) = setup(); - write_decision(dir.path(), "alpha", "body\n"); - write_decision(dir.path(), "beta", "body\n"); - let summary_path = dir.path().join(".yoi/memory/summary.md"); - std::fs::write( - &summary_path, - format!("---\nupdated_at: {n}\n---\nhello\n", n = now()), - ) - .unwrap(); - - let (_, tool) = memory_query_tool(layout, QueryConfig::default())(); - let out = tool.execute("{}", Default::default()).await.unwrap(); - let records: Vec = parse_records(&out); - let mut slugs: Vec<&str> = records.iter().map(|r| r.slug.as_str()).collect(); - slugs.sort(); - assert_eq!(slugs, vec!["alpha", "beta", "summary"]); - // No excerpts when listing. - assert!(records.iter().all(|r| r.excerpt.is_none())); - } - - #[tokio::test] - async fn memory_query_finds_summary() { - let (dir, layout) = setup(); - let summary_path = dir.path().join(".yoi/memory/summary.md"); - std::fs::write( - &summary_path, - format!("---\nupdated_at: {n}\n---\nthe needle is here\n", n = now()), - ) - .unwrap(); - let (_, tool) = memory_query_tool(layout, QueryConfig::default())(); - let inp = serde_json::json!({ "query": "needle" }); - let out = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap(); - let records: Vec = parse_records(&out); - assert_eq!(records.len(), 1); - assert_eq!(records[0].slug, "summary"); - assert_eq!(records[0].kind, "summary"); - } - - #[tokio::test] - async fn query_hits_do_not_log_usage() { - let (dir, layout) = setup(); - write_decision(dir.path(), "alpha", "needle line\n"); - let (_, memory_tool) = memory_query_tool(layout.clone(), QueryConfig::default())(); - let inp = serde_json::json!({ "query": "needle" }); - memory_tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap(); - - let report = crate::usage::build_usage_report(&layout).unwrap(); - assert!(report.records.is_empty()); - assert!(!layout.usage_events_path().exists()); - } - - #[tokio::test] - async fn memory_query_respects_result_limit() { - let (dir, layout) = setup(); - for i in 0..10 { - write_decision(dir.path(), &format!("rec-{i}"), "needle line\n"); - } - let cfg = QueryConfig { - result_limit: 3, - excerpt_lines: 1, - }; - let (_, tool) = memory_query_tool(layout, cfg)(); - let inp = serde_json::json!({ "query": "needle" }); - let out = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap(); - let records: Vec = parse_records(&out); - assert_eq!(records.len(), 3); - } - - #[tokio::test] - async fn memory_query_excerpt_includes_context_lines() { - let (dir, layout) = setup(); - write_decision( - dir.path(), - "ctx", - "line a\nline b\nNEEDLE here\nline d\nline e\n", - ); - let cfg = QueryConfig { - result_limit: 5, - excerpt_lines: 1, - }; - let (_, tool) = memory_query_tool(layout, cfg)(); - let inp = serde_json::json!({ "query": "needle" }); - let out = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap(); - let records: Vec = parse_records(&out); - assert_eq!(records.len(), 1); - let e = records[0].excerpt.as_deref().unwrap(); - assert!(e.contains("line b")); - assert!(e.contains("NEEDLE here")); - assert!(e.contains("line d")); - assert!(!e.contains("line a")); - assert!(!e.contains("line e")); - } - - #[tokio::test] - async fn memory_query_blank_query_rejected() { - let (_dir, layout) = setup(); - let (_, tool) = memory_query_tool(layout, QueryConfig::default())(); - let inp = serde_json::json!({ "query": " " }); - let err = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap_err(); - assert!(matches!(err, ToolError::InvalidArgument(_))); - } -} diff --git a/crates/memory/src/tool/read.rs b/crates/memory/src/tool/read.rs deleted file mode 100644 index 7e05beb2..00000000 --- a/crates/memory/src/tool/read.rs +++ /dev/null @@ -1,327 +0,0 @@ -//! `MemoryRead` tool. -//! -//! Reads a memory record by `(kind, slug)`. Returns -//! line-numbered content (1-based), like the generic Read tool. The -//! agent never names a path — `MemoryQuery` returns `{kind, slug, ...}` -//! and that pair feeds straight into Read. - -use std::sync::Arc; - -use async_trait::async_trait; -use llm_engine::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput}; -use serde::Deserialize; - -use crate::audit::{AuditStatus, RecordUsageAudit, append_record_usage}; -use crate::tool::MemoryToolKind; -use crate::usage::{self, UsageSource}; -use crate::workspace::WorkspaceLayout; - -const DESCRIPTION: &str = "Read a memory record by `kind` + `slug`. \ -`kind` is one of: summary, decision, request. \ -For `summary` omit `slug`; for the others `slug` is required. \ -Returns line-numbered output (1-based)."; - -const DEFAULT_LIMIT: usize = 2000; - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct ReadParams { - /// Record kind: `summary` | `decision` | `request`. - kind: MemoryToolKind, - /// Slug. Required for everything except `summary`; forbidden for `summary`. - #[serde(default)] - slug: Option, - /// 0-based line offset from the start. Defaults to 0. - #[serde(default)] - offset: Option, - /// Maximum number of lines to return. Defaults to 2000. - #[serde(default)] - limit: Option, -} - -struct ReadTool { - layout: WorkspaceLayout, - usage_session_id: Option, -} - -#[async_trait] -impl Tool for ReadTool { - async fn execute( - &self, - input_json: &str, - _ctx: llm_engine::tool::ToolExecutionContext, - ) -> Result { - let params: ReadParams = serde_json::from_str(input_json) - .map_err(|e| ToolError::InvalidArgument(format!("invalid MemoryRead input: {e}")))?; - - let path = params - .kind - .resolve_path(&self.layout, params.slug.as_deref())?; - let kind = params.kind.to_string(); - let slug = audit_slug(¶ms.kind, params.slug.as_deref()); - - let bytes = match std::fs::read(&path) { - Ok(bytes) => bytes, - Err(e) => { - let reason = match e.kind() { - std::io::ErrorKind::NotFound => format!("record not found: {}", path.display()), - _ => format!("read failed at {}: {e}", path.display()), - }; - let _ = append_record_usage( - &self.layout, - RecordUsageAudit { - op: "read".to_string(), - status: AuditStatus::Failed, - kind, - slug: Some(slug), - path: Some(path.display().to_string()), - query: None, - result_count: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::ExecutionFailed(reason)); - } - }; - - let text = String::from_utf8_lossy(&bytes).into_owned(); - if let Some(segment_id) = self.usage_session_id.as_deref() { - let usage_slug = params.slug.as_deref().unwrap_or("summary"); - let snapshot = usage::snapshot_record_from_bytes( - params.kind.record_kind(), - usage_slug.to_string(), - &bytes, - ); - if let Err(err) = usage::append_use_event( - &self.layout, - segment_id.to_string(), - UsageSource::MemoryRead, - vec![snapshot], - ) { - tracing::warn!(error = %err, "failed to append MemoryRead usage event"); - } - } - let offset = params.offset.unwrap_or(0); - let limit = params.limit.unwrap_or(DEFAULT_LIMIT).max(1); - let rendered = render_numbered(&text, offset, limit); - - let summary = if rendered.truncated { - format!( - "Read {} line(s) [{}..{}] of {} from {}", - rendered.line_count, - offset + 1, - offset + rendered.line_count, - rendered.total_lines, - path.display() - ) - } else { - format!( - "Read {} line(s) from {}", - rendered.line_count, - path.display() - ) - }; - - let _ = append_record_usage( - &self.layout, - RecordUsageAudit { - op: "read".to_string(), - status: AuditStatus::Success, - kind, - slug: Some(slug), - path: Some(path.display().to_string()), - query: None, - result_count: Some(rendered.line_count), - reason: if rendered.truncated { - Some("truncated".to_string()) - } else { - None - }, - }, - ); - - Ok(ToolOutput { - summary, - content: Some(rendered.body), - }) - } -} - -fn audit_slug(kind: &MemoryToolKind, slug: Option<&str>) -> String { - match kind { - MemoryToolKind::Summary => "summary".to_string(), - _ => slug.unwrap_or("").to_string(), - } -} - -struct Rendered { - body: String, - line_count: usize, - total_lines: usize, - truncated: bool, -} - -fn render_numbered(text: &str, offset: usize, limit: usize) -> Rendered { - let all_lines: Vec<&str> = text.lines().collect(); - let total_lines = all_lines.len(); - let start = offset.min(total_lines); - let end = start.saturating_add(limit).min(total_lines); - let slice = &all_lines[start..end]; - let line_count = slice.len(); - - use std::fmt::Write as _; - let mut body = String::with_capacity(text.len().saturating_add(line_count * 8)); - for (i, line) in slice.iter().enumerate() { - let lineno = start + i + 1; - let _ = writeln!(&mut body, "{:>6}\t{}", lineno, line); - } - - Rendered { - body, - line_count, - total_lines, - truncated: start > 0 || end < total_lines, - } -} - -pub fn read_tool(layout: WorkspaceLayout) -> ToolDefinition { - read_tool_inner(layout, None) -} - -pub fn read_tool_with_usage( - layout: WorkspaceLayout, - segment_id: impl Into, -) -> ToolDefinition { - read_tool_inner(layout, Some(segment_id.into())) -} - -fn read_tool_inner(layout: WorkspaceLayout, usage_session_id: Option) -> ToolDefinition { - Arc::new(move || { - let schema = schemars::schema_for!(ReadParams); - let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); - let meta = ToolMeta::new("MemoryRead") - .description(DESCRIPTION) - .input_schema(schema_value); - let tool: Arc = Arc::new(ReadTool { - layout: layout.clone(), - usage_session_id: usage_session_id.clone(), - }); - (meta, tool) - }) -} - -#[cfg(test)] -mod tests { - use super::*; - use tempfile::TempDir; - - fn setup() -> (TempDir, WorkspaceLayout) { - let dir = TempDir::new().unwrap(); - let layout = WorkspaceLayout::new(dir.path().to_path_buf()); - (dir, layout) - } - - #[tokio::test] - async fn read_decision_by_slug() { - let (dir, layout) = setup(); - let path = dir.path().join(".yoi/memory/decisions/foo.md"); - std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - std::fs::write(&path, "alpha\nbeta\n").unwrap(); - - let (_meta, tool) = read_tool(layout)(); - let inp = serde_json::json!({ "kind": "decision", "slug": "foo" }); - let out = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap(); - let body = out.content.unwrap(); - assert!(body.contains(" 1\talpha")); - assert!(body.contains(" 2\tbeta")); - } - - #[tokio::test] - async fn read_summary_without_slug() { - let (dir, layout) = setup(); - let path = dir.path().join(".yoi/memory/summary.md"); - std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - std::fs::write(&path, "summary body\n").unwrap(); - - let (_, tool) = read_tool(layout)(); - let inp = serde_json::json!({ "kind": "summary" }); - let out = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap(); - assert!(out.content.unwrap().contains("summary body")); - } - - #[tokio::test] - async fn summary_with_slug_rejected() { - let (_dir, layout) = setup(); - let (_, tool) = read_tool(layout)(); - let inp = serde_json::json!({ "kind": "summary", "slug": "x" }); - let err = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap_err(); - assert!(matches!(err, ToolError::InvalidArgument(_))); - } - - #[tokio::test] - async fn decision_without_slug_rejected() { - let (_dir, layout) = setup(); - let (_, tool) = read_tool(layout)(); - let inp = serde_json::json!({ "kind": "decision" }); - let err = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap_err(); - assert!(matches!(err, ToolError::InvalidArgument(_))); - } - - #[tokio::test] - async fn invalid_slug_rejected() { - let (_dir, layout) = setup(); - let (_, tool) = read_tool(layout)(); - let inp = serde_json::json!({ "kind": "decision", "slug": "Bad-Slug" }); - let err = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap_err(); - assert!(matches!(err, ToolError::InvalidArgument(_))); - } - - #[tokio::test] - async fn read_logs_explicit_use_when_usage_session_is_set() { - let (dir, layout) = setup(); - let path = dir.path().join(".yoi/memory/decisions/foo.md"); - std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - std::fs::write(&path, "alpha\n").unwrap(); - - let (_, tool) = read_tool_with_usage(layout.clone(), "session-1")(); - let inp = serde_json::json!({ "kind": "decision", "slug": "foo" }); - tool.execute(&inp.to_string(), Default::default()) - .await - .unwrap(); - - let report = usage::build_usage_report(&layout).unwrap(); - assert_eq!(report.records.len(), 1); - let record = &report.records[0]; - assert_eq!(record.kind, "decision"); - assert_eq!(record.slug, "foo"); - assert_eq!(record.use_count, 1); - assert_eq!(record.source_breakdown["MemoryRead"], 1); - assert_eq!(record.resident_exposure_count, 0); - } - - #[tokio::test] - async fn missing_file_returns_execution_failed() { - let (_dir, layout) = setup(); - let (_, tool) = read_tool(layout)(); - let inp = serde_json::json!({ "kind": "decision", "slug": "missing" }); - let err = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap_err(); - assert!(matches!(err, ToolError::ExecutionFailed(_))); - } -} diff --git a/crates/memory/src/tool/write.rs b/crates/memory/src/tool/write.rs deleted file mode 100644 index e10f2dbb..00000000 --- a/crates/memory/src/tool/write.rs +++ /dev/null @@ -1,341 +0,0 @@ -//! `MemoryWrite` tool. -//! -//! Creates or overwrites a memory record by `(kind, slug)`. -//! Pre-write Linter validates frontmatter, slug uniqueness (Create only), -//! reference integrity, size limits. On any -//! Linter error the tool returns `ToolError::InvalidArgument` with all -//! violations aggregated and the file is **not** written. - -use std::sync::Arc; - -use async_trait::async_trait; -use llm_engine::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput}; -use serde::Deserialize; - -use crate::audit::{ - AuditStatus, RecordOperationAudit, append_record_operation, file_hash, hash_bytes, -}; -use crate::linter::{LintReport, Linter, WriteMode}; -use crate::tool::MemoryToolKind; -use crate::workspace::WorkspaceLayout; - -const DESCRIPTION: &str = "Create or overwrite a memory record by \ -`kind` + `slug`. `kind`: summary | decision | request. For `summary` \ -omit `slug`. Frontmatter is validated before write; on validation failure no \ -write occurs and every violation is returned in the error message."; - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct WriteParams { - /// Record kind: `summary` | `decision` | `request`. - kind: MemoryToolKind, - /// Slug. Required for everything except `summary`; forbidden for `summary`. - #[serde(default)] - slug: Option, - /// Full file contents (frontmatter + body). - content: String, -} - -struct WriteTool { - layout: WorkspaceLayout, - linter: Linter, -} - -#[async_trait] -impl Tool for WriteTool { - async fn execute( - &self, - input_json: &str, - _ctx: llm_engine::tool::ToolExecutionContext, - ) -> Result { - let params: WriteParams = serde_json::from_str(input_json) - .map_err(|e| ToolError::InvalidArgument(format!("invalid MemoryWrite input: {e}")))?; - - let path = params - .kind - .resolve_path(&self.layout, params.slug.as_deref())?; - let kind = params.kind.to_string(); - let slug = audit_slug(¶ms.kind, params.slug.as_deref()); - - let before_hash = file_hash(&path).ok().flatten(); - let already_exists = before_hash.is_some(); - let mode = if already_exists { - WriteMode::Update - } else { - WriteMode::Create - }; - - let report = self.linter.lint(&path, ¶ms.content, mode); - if report.has_errors() { - let reason = format_report(&report); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "write".to_string(), - status: AuditStatus::Failed, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::InvalidArgument(reason)); - } - - if let Some(parent) = path.parent() { - if let Err(e) = std::fs::create_dir_all(parent) { - let reason = format!("failed to create directory {}: {e}", parent.display()); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "write".to_string(), - status: AuditStatus::Failed, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::ExecutionFailed(reason)); - } - } - if let Err(e) = std::fs::write(&path, params.content.as_bytes()) { - let reason = format!("failed to write {}: {e}", path.display()); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "write".to_string(), - status: AuditStatus::Failed, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash: None, - reason: Some(reason.clone()), - }, - ); - return Err(ToolError::ExecutionFailed(reason)); - } - let after_hash = Some(hash_bytes(params.content.as_bytes())); - let _ = append_record_operation( - &self.layout, - RecordOperationAudit { - op: "write".to_string(), - status: AuditStatus::Success, - kind, - slug, - path: path.display().to_string(), - before_hash, - after_hash, - reason: if report.warnings.is_empty() { - None - } else { - Some(format!("{} warning(s)", report.warnings.len())) - }, - }, - ); - - let summary = format!( - "{} {}{}", - if already_exists { - "Overwrote" - } else { - "Created" - }, - path.display(), - warning_tail(&report), - ); - Ok(ToolOutput { - summary, - content: None, - }) - } -} - -fn audit_slug(kind: &MemoryToolKind, slug: Option<&str>) -> String { - match kind { - MemoryToolKind::Summary => "summary".to_string(), - _ => slug.unwrap_or("").to_string(), - } -} - -fn format_report(report: &LintReport) -> String { - use std::fmt::Write as _; - let mut buf = String::from("memory linter rejected the write:"); - for e in &report.errors { - let _ = write!(&mut buf, "\n - {e}"); - } - if !report.warnings.is_empty() { - let _ = write!(&mut buf, "\nwarnings (informational):"); - for w in &report.warnings { - let _ = write!(&mut buf, "\n - {w}"); - } - } - buf -} - -fn warning_tail(report: &LintReport) -> String { - if report.warnings.is_empty() { - return String::new(); - } - let mut s = format!(" [{} warning(s)]", report.warnings.len()); - for w in &report.warnings { - use std::fmt::Write as _; - let _ = write!(&mut s, " {w};"); - } - s -} - -pub fn write_tool(layout: WorkspaceLayout) -> ToolDefinition { - Arc::new(move || { - let schema = schemars::schema_for!(WriteParams); - let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); - let meta = ToolMeta::new("MemoryWrite") - .description(DESCRIPTION) - .input_schema(schema_value); - let tool: Arc = Arc::new(WriteTool { - layout: layout.clone(), - linter: Linter::new(layout.clone()), - }); - (meta, tool) - }) -} - -#[cfg(test)] -mod tests { - use super::*; - use chrono::Utc; - use tempfile::TempDir; - - fn now() -> String { - Utc::now().to_rfc3339() - } - - fn setup() -> (TempDir, WorkspaceLayout) { - let dir = TempDir::new().unwrap(); - let layout = WorkspaceLayout::new(dir.path().to_path_buf()); - (dir, layout) - } - - #[tokio::test] - async fn write_creates_summary() { - let (dir, layout) = setup(); - let path = dir.path().join(".yoi/memory/summary.md"); - let content = format!("---\nupdated_at: {n}\n---\nbody\n", n = now()); - - let (meta, tool) = write_tool(layout)(); - assert_eq!(meta.name, "MemoryWrite"); - - let inp = serde_json::json!({ - "kind": "summary", - "content": content, - }); - let out = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap(); - assert!(out.summary.contains("Created")); - assert!(path.exists()); - } - - #[tokio::test] - async fn write_aggregates_multiple_errors() { - let (_dir, layout) = setup(); - // Missing required `status` field for decisions. - let huge = "x".repeat(8001); - let content = format!( - "---\ncreated_at: {n}\nupdated_at: {n}\nsources: []\n---\n{huge}", - n = now() - ); - let (_, tool) = write_tool(layout)(); - let inp = serde_json::json!({ - "kind": "decision", - "slug": "foo", - "content": content, - }); - let err = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap_err(); - let msg = format!("{err}"); - assert!(msg.contains("status") || msg.contains("missing"), "{msg}"); - } - - #[tokio::test] - async fn write_update_existing() { - let (dir, layout) = setup(); - let path = dir.path().join(".yoi/memory/decisions/foo.md"); - std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - let initial = format!( - "---\ncreated_at: {n}\nupdated_at: {n}\nsources: []\nstatus: open\n---\nold\n", - n = now() - ); - std::fs::write(&path, &initial).unwrap(); - - let (_, tool) = write_tool(layout.clone())(); - let inp = serde_json::json!({ - "kind": "decision", - "slug": "foo", - "content": initial, - }); - let out = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap(); - assert!(out.summary.contains("Overwrote")); - } - - #[tokio::test] - async fn write_decision_requires_slug() { - let (_dir, layout) = setup(); - let (_, tool) = write_tool(layout)(); - let inp = serde_json::json!({ - "kind": "decision", - "content": "ignored", - }); - let err = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap_err(); - assert!(matches!(err, ToolError::InvalidArgument(_))); - } - - #[tokio::test] - async fn write_does_not_persist_on_lint_failure() { - let (dir, layout) = setup(); - let path = dir.path().join(".yoi/memory/decisions/foo.md"); - let bad = "no frontmatter at all"; - let (_, tool) = write_tool(layout)(); - let inp = serde_json::json!({ - "kind": "decision", - "slug": "foo", - "content": bad, - }); - assert!( - tool.execute(&inp.to_string(), Default::default()) - .await - .is_err() - ); - assert!(!path.exists()); - } - - #[tokio::test] - async fn workflow_kind_not_acceptable() { - // The MemoryToolKind enum doesn't include Workflow, so deserialization fails. - let (_dir, layout) = setup(); - let (_, tool) = write_tool(layout)(); - let inp = serde_json::json!({ - "kind": "workflow", - "slug": "wf", - "content": "---\n---\n", - }); - let err = tool - .execute(&inp.to_string(), Default::default()) - .await - .unwrap_err(); - assert!(matches!(err, ToolError::InvalidArgument(_))); - } -} diff --git a/crates/worker/src/feature/builtin/memory.rs b/crates/worker/src/feature/builtin/memory.rs index 045bde3c..4e034fa9 100644 --- a/crates/worker/src/feature/builtin/memory.rs +++ b/crates/worker/src/feature/builtin/memory.rs @@ -324,7 +324,7 @@ fn tool_output(output: MemoryToolOutput) -> ToolOutput { const READ_DOCUMENT_DESCRIPTION: &str = "Read the Workspace memory Markdown document through Workspace authority."; const UPDATE_DOCUMENT_DESCRIPTION: &str = - "Replace the Workspace memory Markdown document through Workspace authority."; + "Edit the Workspace memory Markdown document by replacing an exact old_string with new_string."; const QUERY_DESCRIPTION: &str = "Query the Workspace memory document through Workspace authority."; const STAGING_LIST_DESCRIPTION: &str = "List pending Memory staging candidates without loading full record payloads."; @@ -346,9 +346,11 @@ fn document_update_schema() -> serde_json::Value { json!({ "type":"object", "additionalProperties": false, - "required":["body_md"], + "required":["old_string", "new_string"], "properties":{ - "body_md":{"type":"string"} + "old_string":{"type":"string", "minLength": 1}, + "new_string":{"type":"string"}, + "replace_all":{"type":"boolean", "default": false} } }) } @@ -377,6 +379,15 @@ mod tests { names } + fn tool_meta(definitions: Vec, name: &str) -> serde_json::Value { + definitions + .into_iter() + .map(|tool| tool().0) + .find(|meta| meta.name == name) + .unwrap_or_else(|| panic!("missing tool meta for {name}")) + .input_schema + } + #[test] fn normal_workspace_memory_tools_do_not_include_staging_tools() { let names = tool_names(workspace_http_memory_tools( @@ -396,6 +407,37 @@ mod tests { assert!(!names.contains(&"MemoryStagingClose".to_string())); } + #[test] + fn document_update_schema_is_edit_like_and_staging_close_has_no_legacy_kinds() { + let update_schema = tool_meta( + workspace_http_memory_tools("workspace".to_string(), "http://backend".to_string()), + "MemoryUpdateDocument", + ); + assert_eq!( + update_schema["required"], + serde_json::json!(["old_string", "new_string"]) + ); + assert!(update_schema["properties"].get("old_string").is_some()); + assert!(update_schema["properties"].get("new_string").is_some()); + assert!(update_schema["properties"].get("replace_all").is_some()); + assert!(update_schema["properties"].get("body_md").is_none()); + + let close_schema_text = tool_meta( + workspace_http_memory_consolidation_tools( + "workspace".to_string(), + "http://backend".to_string(), + ), + "MemoryStagingClose", + ) + .to_string(); + assert!(close_schema_text.contains("affected_memory")); + assert!(close_schema_text.contains("edit")); + assert!(!close_schema_text.contains("summary")); + assert!(!close_schema_text.contains("decision")); + assert!(!close_schema_text.contains("request")); + assert!(!close_schema_text.contains("slug")); + } + #[test] fn consolidation_workspace_memory_tools_include_staging_tools() { let names = tool_names(workspace_http_memory_consolidation_tools( diff --git a/crates/worker/src/prompt/catalog.rs b/crates/worker/src/prompt/catalog.rs index fad3452a..4d1be72d 100644 --- a/crates/worker/src/prompt/catalog.rs +++ b/crates/worker/src/prompt/catalog.rs @@ -541,6 +541,28 @@ mod tests { assert!(consolidate.contains("`language`: `Japanese`")); } + #[test] + fn memory_consolidation_prompt_describes_document_edit_policy_without_storage_details() { + let cat = PromptCatalog::builtins_only().unwrap(); + let consolidate = cat.memory_consolidation_system("Japanese").unwrap(); + assert!(consolidate.contains("Durable Memory is one Markdown document")); + assert!(consolidate.contains("existing `##` section")); + assert!(consolidate.contains("MemoryUpdateDocument")); + assert!(consolidate.contains("old_string")); + assert!(consolidate.contains("new_string")); + assert!(consolidate.contains("MemoryStagingClose")); + assert!(consolidate.contains("`applied`")); + assert!(consolidate.contains(r#"{"operation":"edit"}"#)); + assert!(consolidate.contains("Do not create Knowledge, Skill, Ticket")); + assert!(consolidate.contains("Do not create separate categorized Memory records")); + assert!(!consolidate.contains("SQLite")); + assert!(!consolidate.contains("SQL")); + assert!(!consolidate.contains("storage")); + assert!(!consolidate.contains("decision record")); + assert!(!consolidate.contains("request record")); + assert!(!consolidate.contains("summary record")); + } + #[test] fn notify_wrapper_interpolates_message() { let cat = PromptCatalog::builtins_only().unwrap(); diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 5b5dba0e..2434d06b 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -528,7 +528,7 @@ mod tests { let staging = authority .upsert_memory_staging_record( "candidate-1", - r#"{"kind":"summary","body_md":"Candidate"}"#, + r#"{"claim":"Candidate"}"#, Some("memory/_staging/candidate-1.json"), ) .unwrap(); diff --git a/crates/workspace-server/src/memory_backend.rs b/crates/workspace-server/src/memory_backend.rs index bd0d922f..8d6b38ff 100644 --- a/crates/workspace-server/src/memory_backend.rs +++ b/crates/workspace-server/src/memory_backend.rs @@ -1,9 +1,8 @@ use memory::backend::{ MemoryBackendAckOutput, MemoryBackendOperation, MemoryBackendOperationResult, MemoryDocumentReadOperation, MemoryDocumentUpdateOperation, MemoryQueryOperation, - MemoryReadOperation, MemoryStagingCloseAction, MemoryStagingCloseOperation, - MemoryStagingListOperation, MemoryStagingReadOperation, MemoryStagingWriteOutput, - MemoryToolOutput, + MemoryStagingCloseAction, MemoryStagingCloseOperation, MemoryStagingListOperation, + MemoryStagingReadOperation, MemoryStagingWriteOutput, MemoryToolOutput, }; use memory::extract::{ExtractedCandidate, StagingEvidence, StagingRecord}; use memory::schema::{SourceEvidenceRef, SourceRef}; @@ -24,10 +23,13 @@ pub fn execute_memory_backend_operation_with_authority( MemoryBackendOperation::Query(operation) => { execute_query(authority, operation).map(MemoryBackendOperationResult::ToolOutput) } - MemoryBackendOperation::ReadDocument(operation) => execute_read_document(authority, operation) - .map(MemoryBackendOperationResult::ToolOutput), + MemoryBackendOperation::ReadDocument(operation) => { + execute_read_document(authority, operation) + .map(MemoryBackendOperationResult::ToolOutput) + } MemoryBackendOperation::UpdateDocument(operation) => { - execute_update_document(authority, operation).map(MemoryBackendOperationResult::ToolOutput) + execute_update_document(authority, operation) + .map(MemoryBackendOperationResult::ToolOutput) } MemoryBackendOperation::ResidentSummary(_operation) => Ok( MemoryBackendOperationResult::ToolOutput(execute_resident_summary(authority)?), @@ -73,19 +75,16 @@ pub fn execute_memory_backend_operation_with_authority( }, )) } - MemoryBackendOperation::StagingList(operation) => execute_staging_list(authority, operation) - .map(MemoryBackendOperationResult::ToolOutput), - MemoryBackendOperation::StagingRead(operation) => execute_staging_read(authority, operation) - .map(MemoryBackendOperationResult::ToolOutput), - MemoryBackendOperation::StagingClose(operation) => execute_staging_close(authority, operation) - .map(MemoryBackendOperationResult::ToolOutput), - MemoryBackendOperation::Read(operation) => execute_legacy_read(authority, operation) - .map(MemoryBackendOperationResult::ToolOutput), - MemoryBackendOperation::Write(_) - | MemoryBackendOperation::Edit(_) - | MemoryBackendOperation::Delete(_) => Err(Error::Store( - "legacy summary/decision/request Memory mutation operations are not supported by Workspace authority; use MemoryUpdateDocument".to_string(), - )), + MemoryBackendOperation::StagingList(operation) => { + execute_staging_list(authority, operation).map(MemoryBackendOperationResult::ToolOutput) + } + MemoryBackendOperation::StagingRead(operation) => { + execute_staging_read(authority, operation).map(MemoryBackendOperationResult::ToolOutput) + } + MemoryBackendOperation::StagingClose(operation) => { + execute_staging_close(authority, operation) + .map(MemoryBackendOperationResult::ToolOutput) + } } } @@ -135,24 +134,6 @@ fn execute_query( } } -fn execute_legacy_read( - authority: &A, - operation: MemoryReadOperation, -) -> Result { - if operation.kind.as_str() != "summary" || operation.slug.is_some() { - return Err(Error::Store( - "legacy kind/slug Memory reads are not supported by Workspace authority; use MemoryReadDocument".to_string(), - )); - } - execute_read_document( - authority, - MemoryDocumentReadOperation { - offset: operation.offset, - limit: operation.limit, - }, - ) -} - fn execute_read_document( authority: &A, operation: MemoryDocumentReadOperation, @@ -185,18 +166,57 @@ fn execute_update_document( authority: &A, operation: MemoryDocumentUpdateOperation, ) -> Result { - if operation.body_md.trim().is_empty() { + if operation.old_string == operation.new_string { return Err(Error::Store( - "memory document body_md must not be empty".to_string(), + "memory document old_string and new_string must differ".to_string(), )); } - let document = authority.update_memory_document(&operation.body_md)?; + if operation.old_string.is_empty() { + return Err(Error::Store( + "memory document old_string must not be empty".to_string(), + )); + } + let document = authority.ensure_memory_document()?; + let match_count = document.body_md.matches(&operation.old_string).count(); + if match_count == 0 { + return Err(Error::Store( + "memory document old_string was not found".to_string(), + )); + } + if match_count > 1 && !operation.replace_all { + return Err(Error::Store( + "memory document old_string matched more than once; set replace_all=true to replace every occurrence".to_string(), + )); + } + let updated_body = if operation.replace_all { + document + .body_md + .replace(&operation.old_string, &operation.new_string) + } else { + document + .body_md + .replacen(&operation.old_string, &operation.new_string, 1) + }; + if updated_body.trim().is_empty() { + return Err(Error::Store( + "memory document body must not become empty".to_string(), + )); + } + let updated = authority.update_memory_document(&updated_body)?; Ok(MemoryToolOutput { summary: format!( - "Updated Workspace memory document ({} bytes)", - document.body_md.len() + "Edited {} occurrence(s) in Workspace memory document", + if operation.replace_all { + match_count + } else { + 1 + } ), - content: Some(format!("updated_at={}", document.updated_at)), + content: Some(format!( + "updated_at={} bytes={}", + updated.updated_at, + updated.body_md.len() + )), }) } @@ -409,7 +429,9 @@ mod tests { execute_memory_backend_operation_with_authority( &authority, MemoryBackendOperation::UpdateDocument(MemoryDocumentUpdateOperation { - body_md: "# Memory\n\nSQLite backed body".into(), + old_string: "# Memory\n\n".into(), + new_string: "# Memory\n\nSQLite backed body".into(), + replace_all: false, }), ) .unwrap(); diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index a62ac803..4a99256c 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -3113,7 +3113,7 @@ mod tests { let staging = MemoryStagingRecord { workspace_id: "local-dev".to_string(), candidate_id: "candidate-a".to_string(), - raw_json: r#"{"kind":"summary","body":"candidate"}"#.to_string(), + raw_json: r#"{"claim":"candidate"}"#.to_string(), source_path: Some("memory/_staging/candidate-a.json".to_string()), imported_at: "4".to_string(), }; diff --git a/resources/prompts/internal/memory_consolidation_system.md b/resources/prompts/internal/memory_consolidation_system.md index 1d2d21f8..b007cc86 100644 --- a/resources/prompts/internal/memory_consolidation_system.md +++ b/resources/prompts/internal/memory_consolidation_system.md @@ -9,24 +9,41 @@ Use this loop: 1. Call `MemoryStagingList` to inspect pending candidates. 2. Pick one candidate and call `MemoryStagingRead` for the full record. 3. Use `MemoryQuery` / `MemoryReadDocument` to compare against durable Memory. -4. If the candidate should change durable Memory, update the single durable Markdown document with `MemoryUpdateDocument` first. +4. If the candidate should change durable Memory, edit the durable Markdown document with `MemoryUpdateDocument` before closing the candidate. 5. Call `MemoryStagingClose` exactly once for every candidate you finish. Always include a concrete reason. +Durable Memory is one Markdown document. Keep it concise and useful for future workers: + +- Merge facts into the most relevant existing `##` section when possible. +- Add a new `##` section only when the candidate does not fit an existing section. +- Prefer short bullets or short paragraphs over long transcripts. +- Preserve exact identifiers, paths, commands, Ticket IDs, branch names, and quoted user text when they matter. +- Do not create separate categorized Memory records. + +`MemoryUpdateDocument` edits the document by exact replacement, like a text edit tool: + +- Read enough context first with `MemoryReadDocument`. +- Provide the exact `old_string` to replace and the desired `new_string`. +- Use `replace_all` only when every occurrence should change. +- Do not rewrite the whole document unless the whole document is the intended `old_string`. + `MemoryStagingClose` removes the staging candidate after recording your disposition. Use it for both applied candidates and candidates that should not become Memory. Valid close actions: -- `applied`: you changed durable Memory for this candidate. +- `applied`: you edited durable Memory for this candidate. Include `affected_memory: [{"operation":"edit"}]`. - `discarded`: the candidate is understandable but should not become durable Memory. - `invalid`: the staging record is malformed or cannot be interpreted. -- `duplicate`: the candidate duplicates another candidate or record. +- `duplicate`: the candidate duplicates another candidate or durable Memory. - `already_covered`: durable Memory already covers the candidate sufficiently. +Close reasons must be specific. For example, say what was added or updated for `applied`, what existing section already covered it for `already_covered`, why it is not durable enough for `discarded`, or what is malformed for `invalid`. + ## Language - `language`: `{{language}}` -- Write durable memory prose in this language, including frontmatter descriptions and record bodies. +- Write durable memory prose in this language. - Preserve literal identifiers, paths, commands, branch names, issue IDs, tool names, model names, and quoted user/system text as-is. - If the configured language is unclear, use English. -Do not create Tickets, documents, child Workers, or filesystem files. Use only Memory and MemoryStaging tools. +Do not create Knowledge, Skill, Ticket, documentation, child Worker, or filesystem records. Use only Memory and MemoryStaging tools.