refactor: unify Memory feature configuration authority
This commit is contained in:
@@ -901,27 +901,39 @@ pub(crate) fn wire_event_bridges_on_engine<C, St>(
|
||||
// per-item commit channel is wired at the top of this function.
|
||||
}
|
||||
|
||||
fn add_memory_lifecycle_if_configured<M>(
|
||||
fn add_memory_tools_if_configured<M>(
|
||||
registry: &mut FeatureRegistryBuilder,
|
||||
config: Option<manifest::MemoryConfig>,
|
||||
workspace_bound: bool,
|
||||
build: impl FnOnce(manifest::MemoryConfig) -> std::io::Result<M>,
|
||||
config: &manifest::ResolvedMemoryFeatureConfig,
|
||||
build: impl FnOnce() -> std::io::Result<M>,
|
||||
) -> std::io::Result<bool>
|
||||
where
|
||||
M: crate::feature::FeatureModule + 'static,
|
||||
{
|
||||
let Some(config) = config else {
|
||||
return Ok(false);
|
||||
};
|
||||
if config.workspace_settings().is_none() {
|
||||
if workspace_bound {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"Workspace-bound Memory requires a Backend-authored settings snapshot",
|
||||
));
|
||||
}
|
||||
if !config.profile.enabled {
|
||||
return Ok(false);
|
||||
}
|
||||
config
|
||||
.validate_execution()
|
||||
.map_err(|message| std::io::Error::new(std::io::ErrorKind::InvalidInput, message))?;
|
||||
registry.add_module(build()?);
|
||||
Ok(true)
|
||||
}
|
||||
|
||||
fn add_memory_lifecycle_if_configured<M>(
|
||||
registry: &mut FeatureRegistryBuilder,
|
||||
config: manifest::ResolvedMemoryFeatureConfig,
|
||||
lifecycle_enabled: bool,
|
||||
build: impl FnOnce(manifest::ResolvedMemoryFeatureConfig) -> std::io::Result<M>,
|
||||
) -> std::io::Result<bool>
|
||||
where
|
||||
M: crate::feature::FeatureModule + 'static,
|
||||
{
|
||||
if !lifecycle_enabled || !config.profile.enabled || !config.profile.extraction.enabled {
|
||||
return Ok(false);
|
||||
}
|
||||
config
|
||||
.validate_execution()
|
||||
.map_err(|message| std::io::Error::new(std::io::ErrorKind::InvalidInput, message))?;
|
||||
registry.add_module(build(config)?);
|
||||
Ok(true)
|
||||
}
|
||||
@@ -962,7 +974,7 @@ where
|
||||
let local_filesystem = worker.local_working_directory().cloned();
|
||||
let local_workspace_root = local_filesystem.as_ref().map(|local| local.root.clone());
|
||||
let task_feature = worker.task_feature();
|
||||
let memory_config = worker.manifest().memory.clone();
|
||||
let memory_config = feature_config.memory.clone();
|
||||
let web_config = worker.manifest().web.clone();
|
||||
let mcp_config = worker.manifest().mcp.clone();
|
||||
let spawner_name = worker.manifest().worker.name.clone();
|
||||
@@ -1019,13 +1031,23 @@ where
|
||||
let worker_enabled = feature_config.worker.enabled;
|
||||
let sub_worker_enabled = feature_config.sub_worker.enabled;
|
||||
let mut feature_registry = FeatureRegistryBuilder::new();
|
||||
add_memory_tools_if_configured(&mut feature_registry, &memory_config, || {
|
||||
let workspace_client = worker.workspace_client_handle();
|
||||
if !workspace_client.is_available() || workspace_client.workspace_id().is_none() {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"Memory tools require Backend Workspace API authority",
|
||||
));
|
||||
}
|
||||
Ok(crate::feature::builtin::memory::MemoryToolsFeature::new(
|
||||
workspace_client,
|
||||
memory_config.profile.staging_tools,
|
||||
))
|
||||
})?;
|
||||
add_memory_lifecycle_if_configured(
|
||||
&mut feature_registry,
|
||||
worker
|
||||
.manifest_lifecycle_features_enabled()
|
||||
.then(|| memory_config.clone())
|
||||
.flatten(),
|
||||
spawner_workspace_context.workspace_id().is_some(),
|
||||
memory_config.clone(),
|
||||
worker.manifest_lifecycle_features_enabled(),
|
||||
|config| {
|
||||
let workspace_client = worker.workspace_client_handle();
|
||||
if !workspace_client.is_available() || workspace_client.workspace_id().is_none() {
|
||||
@@ -1202,38 +1224,6 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
// Memory tools require explicit feature exposure. Workspace memory access
|
||||
// is authority-bound to the Backend Workspace API; the Worker must not
|
||||
// register local filesystem memory tools even when it has local cwd/root
|
||||
// authority for shell/file tools.
|
||||
if feature_config.memory.enabled {
|
||||
let _mem = memory_config.as_ref().ok_or_else(|| {
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"[feature.memory].enabled = true requires a [memory] configuration section",
|
||||
)
|
||||
})?;
|
||||
if workspace_client.is_available() && workspace_client.workspace_id().is_some() {
|
||||
let definitions = if feature_config.memory.staging {
|
||||
crate::feature::builtin::memory::workspace_http_memory_consolidation_tools(
|
||||
workspace_client.clone(),
|
||||
)
|
||||
} else {
|
||||
crate::feature::builtin::memory::workspace_http_memory_tools(
|
||||
workspace_client.clone(),
|
||||
)
|
||||
};
|
||||
for definition in definitions {
|
||||
engine.register_tool(definition);
|
||||
}
|
||||
} else {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"memory tools require Backend Workspace API authority",
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
let mut observation_providers: Vec<
|
||||
Arc<dyn crate::feature::builtin::worker_observation::WorkerObservationProvider>,
|
||||
> = Vec::new();
|
||||
@@ -2169,7 +2159,7 @@ mod tests {
|
||||
use tokio::net::UnixListener;
|
||||
|
||||
#[test]
|
||||
fn memory_lifecycle_registration_requires_bound_workspace_memory_config() {
|
||||
fn memory_feature_registration_requires_bound_workspace_memory_config() {
|
||||
#[derive(Clone)]
|
||||
struct TestMemoryLifecycleModule;
|
||||
|
||||
@@ -2189,16 +2179,43 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
let mut registry = FeatureRegistryBuilder::new();
|
||||
let installed = add_memory_tools_if_configured::<TestMemoryLifecycleModule>(
|
||||
&mut registry,
|
||||
&manifest::ResolvedMemoryFeatureConfig::default(),
|
||||
|| panic!("disabled Memory must not construct its tools Feature"),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(!installed);
|
||||
|
||||
let mut missing_snapshot = manifest::ResolvedMemoryFeatureConfig::default();
|
||||
missing_snapshot.profile.enabled = true;
|
||||
let error = add_memory_tools_if_configured::<TestMemoryLifecycleModule>(
|
||||
&mut registry,
|
||||
&missing_snapshot,
|
||||
|| panic!("invalid Memory config must fail before tools Feature construction"),
|
||||
)
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
error
|
||||
.to_string()
|
||||
.contains("requires trusted Workspace settings")
|
||||
);
|
||||
|
||||
let mut registry = FeatureRegistryBuilder::new();
|
||||
let configured = std::cell::Cell::new(false);
|
||||
let mut memory_config = manifest::MemoryConfig::default();
|
||||
memory_config.bind_workspace_settings(&manifest::WorkspaceMemorySettingsSnapshot {
|
||||
workspace_id: "workspace-1".to_string(),
|
||||
settings_revision: 1,
|
||||
language: "English".to_string(),
|
||||
});
|
||||
let mut memory_config = manifest::ResolvedMemoryFeatureConfig::default();
|
||||
memory_config.profile.enabled = true;
|
||||
memory_config.profile.extraction.enabled = true;
|
||||
memory_config
|
||||
.bind_workspace_settings(manifest::WorkspaceMemorySettingsSnapshot {
|
||||
workspace_id: "workspace-1".to_string(),
|
||||
settings_revision: 1,
|
||||
language: "English".to_string(),
|
||||
})
|
||||
.unwrap();
|
||||
let installed =
|
||||
add_memory_lifecycle_if_configured(&mut registry, Some(memory_config), true, |_| {
|
||||
add_memory_lifecycle_if_configured(&mut registry, memory_config, true, |_| {
|
||||
configured.set(true);
|
||||
Ok(TestMemoryLifecycleModule)
|
||||
})
|
||||
@@ -2209,25 +2226,38 @@ mod tests {
|
||||
let mut registry = FeatureRegistryBuilder::new();
|
||||
let installed = add_memory_lifecycle_if_configured::<TestMemoryLifecycleModule>(
|
||||
&mut registry,
|
||||
None,
|
||||
false,
|
||||
manifest::ResolvedMemoryFeatureConfig::default(),
|
||||
true,
|
||||
|_| panic!("disabled Memory must not construct its lifecycle Feature"),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(!installed);
|
||||
|
||||
let mut lifecycle_disabled = manifest::ResolvedMemoryFeatureConfig::default();
|
||||
lifecycle_disabled.profile.enabled = true;
|
||||
lifecycle_disabled.profile.extraction.enabled = true;
|
||||
lifecycle_disabled
|
||||
.bind_workspace_settings(manifest::WorkspaceMemorySettingsSnapshot {
|
||||
workspace_id: "workspace-1".to_string(),
|
||||
settings_revision: 1,
|
||||
language: "English".to_string(),
|
||||
})
|
||||
.unwrap();
|
||||
let installed = add_memory_lifecycle_if_configured::<TestMemoryLifecycleModule>(
|
||||
&mut registry,
|
||||
Some(manifest::MemoryConfig::default()),
|
||||
lifecycle_disabled,
|
||||
false,
|
||||
|_| panic!("Memory without a Backend-authored settings snapshot must stay disabled"),
|
||||
|_| panic!("disabled lifecycle must not construct its Feature"),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(!installed);
|
||||
|
||||
let mut missing_snapshot = manifest::ResolvedMemoryFeatureConfig::default();
|
||||
missing_snapshot.profile.enabled = true;
|
||||
missing_snapshot.profile.extraction.enabled = true;
|
||||
let error = add_memory_lifecycle_if_configured::<TestMemoryLifecycleModule>(
|
||||
&mut registry,
|
||||
Some(manifest::MemoryConfig::default()),
|
||||
missing_snapshot,
|
||||
true,
|
||||
|_| panic!("invalid Workspace Memory config must fail before Feature construction"),
|
||||
)
|
||||
@@ -2235,7 +2265,7 @@ mod tests {
|
||||
assert!(
|
||||
error
|
||||
.to_string()
|
||||
.contains("Backend-authored settings snapshot")
|
||||
.contains("requires trusted Workspace settings")
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -18,6 +18,10 @@ use schemars::JsonSchema;
|
||||
use serde::de::DeserializeOwned;
|
||||
use serde_json::json;
|
||||
|
||||
use crate::feature::{
|
||||
FeatureDescriptor, FeatureInstallContext, FeatureInstallError, FeatureModule, ToolContribution,
|
||||
ToolDeclaration,
|
||||
};
|
||||
use crate::worker::{
|
||||
WorkspaceClient, WorkspaceClientError, WorkspaceRequest, WorkspaceRequestMethod,
|
||||
};
|
||||
@@ -338,6 +342,44 @@ fn query_schema() -> serde_json::Value {
|
||||
})
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct MemoryToolsFeature {
|
||||
tools: Vec<ToolDefinition>,
|
||||
}
|
||||
|
||||
impl MemoryToolsFeature {
|
||||
pub fn new(client: Arc<dyn WorkspaceClient>, staging_tools: bool) -> Self {
|
||||
let tools = if staging_tools {
|
||||
workspace_http_memory_consolidation_tools(client)
|
||||
} else {
|
||||
workspace_http_memory_tools(client)
|
||||
};
|
||||
Self { tools }
|
||||
}
|
||||
}
|
||||
|
||||
impl FeatureModule for MemoryToolsFeature {
|
||||
fn descriptor(&self) -> FeatureDescriptor {
|
||||
let mut descriptor = FeatureDescriptor::builtin("memory", "Memory")
|
||||
.with_description("Workspace Memory document, query, and staging tools.");
|
||||
for tool in &self.tools {
|
||||
let (meta, _) = tool();
|
||||
descriptor = descriptor.with_tool(ToolDeclaration::new(meta.name, meta.description));
|
||||
}
|
||||
descriptor
|
||||
}
|
||||
|
||||
fn install(&self, context: &mut FeatureInstallContext<'_>) -> Result<(), FeatureInstallError> {
|
||||
for tool in &self.tools {
|
||||
let (meta, _) = tool();
|
||||
context
|
||||
.tools()
|
||||
.register(ToolContribution::new(meta.name, tool.clone()))?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -368,6 +410,20 @@ mod tests {
|
||||
.input_schema
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn memory_feature_owns_normal_and_staging_tool_surfaces() {
|
||||
let normal = MemoryToolsFeature::new(test_client(), false);
|
||||
let normal_names = tool_names(normal.tools);
|
||||
assert!(normal_names.contains(&"MemoryQuery".to_string()));
|
||||
assert!(!normal_names.contains(&"MemoryStagingList".to_string()));
|
||||
|
||||
let staging = MemoryToolsFeature::new(test_client(), true);
|
||||
assert_eq!(staging.descriptor().id.as_str(), "builtin:memory");
|
||||
let staging_names = tool_names(staging.tools);
|
||||
assert!(staging_names.contains(&"MemoryQuery".to_string()));
|
||||
assert!(staging_names.contains(&"MemoryStagingList".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normal_workspace_memory_tools_do_not_include_staging_tools() {
|
||||
let names = tool_names(workspace_http_memory_tools(test_client()));
|
||||
|
||||
@@ -52,7 +52,7 @@ pub(crate) struct MemoryLifecycleFeature {
|
||||
|
||||
#[derive(Clone)]
|
||||
struct MemoryLifecycleTask {
|
||||
config: manifest::MemoryConfig,
|
||||
config: manifest::ResolvedMemoryFeatureConfig,
|
||||
capture: CommittedSessionCaptureHandle,
|
||||
extensions: SessionExtensionHandle,
|
||||
workspace_client: Arc<dyn WorkspaceClient>,
|
||||
@@ -66,7 +66,7 @@ struct MemoryLifecycleTask {
|
||||
impl MemoryLifecycleFeature {
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) fn new(
|
||||
config: manifest::MemoryConfig,
|
||||
config: manifest::ResolvedMemoryFeatureConfig,
|
||||
capture: CommittedSessionCaptureHandle,
|
||||
extensions: SessionExtensionHandle,
|
||||
workspace_client: Arc<dyn WorkspaceClient>,
|
||||
@@ -132,7 +132,9 @@ impl MemoryLifecycleTask {
|
||||
memory::audit::AuditWorker::MemoryExtract,
|
||||
memory::audit::AuditTrigger::TokenThreshold,
|
||||
self.config
|
||||
.extract_model
|
||||
.profile
|
||||
.extraction
|
||||
.model
|
||||
.as_ref()
|
||||
.or(Some(&self.manifest.model))
|
||||
.map(model_audit_from_manifest),
|
||||
@@ -188,7 +190,9 @@ impl MemoryLifecycleTask {
|
||||
};
|
||||
let Some(threshold) = self
|
||||
.config
|
||||
.extract_threshold
|
||||
.profile
|
||||
.extraction
|
||||
.threshold
|
||||
.filter(|threshold| *threshold > 0)
|
||||
else {
|
||||
audit
|
||||
@@ -283,7 +287,7 @@ impl MemoryLifecycleTask {
|
||||
source,
|
||||
audit.run_id.to_string(),
|
||||
);
|
||||
let client = if let Some(model) = self.config.extract_model.as_ref() {
|
||||
let client = if let Some(model) = self.config.profile.extraction.model.as_ref() {
|
||||
match crate::model_client::build_client(model) {
|
||||
Ok(client) => client,
|
||||
Err(error) => {
|
||||
@@ -321,7 +325,7 @@ impl MemoryLifecycleTask {
|
||||
}
|
||||
};
|
||||
let mut manifest = self.manifest.clone();
|
||||
if let Some(model) = self.config.extract_model.clone() {
|
||||
if let Some(model) = self.config.profile.extraction.model.clone() {
|
||||
manifest.model = model;
|
||||
}
|
||||
|
||||
@@ -349,7 +353,9 @@ impl MemoryLifecycleTask {
|
||||
cache_key: Some(capture.segment_id.clone()),
|
||||
max_turns: self
|
||||
.config
|
||||
.extract_worker_max_turns
|
||||
.profile
|
||||
.extraction
|
||||
.worker_max_turns
|
||||
.or(manifest::defaults::MEMORY_EXTRACT_WORKER_MAX_TURNS),
|
||||
engine_configurator: None,
|
||||
features,
|
||||
@@ -493,36 +499,13 @@ impl MemoryLifecycleTask {
|
||||
let audit = WorkerAuditBase::new(
|
||||
memory::audit::AuditWorker::MemoryConsolidation,
|
||||
memory::audit::AuditTrigger::StagingBacklog,
|
||||
self.config
|
||||
.consolidation_model
|
||||
.as_ref()
|
||||
.or(Some(&self.manifest.model))
|
||||
.map(model_audit_from_manifest),
|
||||
Some(model_audit_from_manifest(&self.manifest.model)),
|
||||
)
|
||||
.with_memory_settings(&self.config);
|
||||
let Some((threshold_files, threshold_bytes)) = consolidation_thresholds(&self.config)
|
||||
else {
|
||||
audit
|
||||
.emit(
|
||||
self.workspace_client.as_ref(),
|
||||
self.event_tx.as_ref(),
|
||||
memory::audit::WorkerLifecycleStatus::Skipped,
|
||||
"consolidation_threshold_disabled",
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
return;
|
||||
};
|
||||
match self
|
||||
.workspace_client
|
||||
.request_memory_staging_consolidation(
|
||||
memory::backend::MemoryConsolidateStagingOperation {
|
||||
force: false,
|
||||
threshold_files,
|
||||
threshold_bytes,
|
||||
},
|
||||
memory::backend::MemoryConsolidateStagingOperation { force: false },
|
||||
)
|
||||
.await
|
||||
{
|
||||
@@ -646,22 +629,6 @@ fn extract_pointer(
|
||||
Ok(pointer)
|
||||
}
|
||||
|
||||
fn consolidation_thresholds(
|
||||
config: &manifest::MemoryConfig,
|
||||
) -> Option<(Option<usize>, Option<u64>)> {
|
||||
let threshold_files = config
|
||||
.consolidation_threshold_files
|
||||
.filter(|threshold| *threshold > 0);
|
||||
let threshold_bytes = config
|
||||
.consolidation_threshold_bytes
|
||||
.filter(|threshold| *threshold > 0);
|
||||
if threshold_files.is_none() && threshold_bytes.is_none() {
|
||||
None
|
||||
} else {
|
||||
Some((threshold_files, threshold_bytes))
|
||||
}
|
||||
}
|
||||
|
||||
fn extraction_run_eligible(exit: CommittedRunExit) -> bool {
|
||||
exit == CommittedRunExit::Finished
|
||||
}
|
||||
@@ -688,12 +655,17 @@ fn tokens_since_pointer(
|
||||
fn extraction_threshold_reached(
|
||||
capture: &CommittedSessionCapture,
|
||||
pointer: Option<&memory::ExtractPointerPayload>,
|
||||
config: &manifest::MemoryConfig,
|
||||
config: &manifest::ResolvedMemoryFeatureConfig,
|
||||
) -> bool {
|
||||
if capture.history.is_empty() {
|
||||
return false;
|
||||
}
|
||||
let Some(threshold) = config.extract_threshold.filter(|threshold| *threshold > 0) else {
|
||||
let Some(threshold) = config
|
||||
.profile
|
||||
.extraction
|
||||
.threshold
|
||||
.filter(|threshold| *threshold > 0)
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
tokens_since_pointer(capture, pointer) >= threshold
|
||||
@@ -723,7 +695,7 @@ impl WorkerAuditBase {
|
||||
}
|
||||
}
|
||||
|
||||
fn with_memory_settings(mut self, config: &manifest::MemoryConfig) -> Self {
|
||||
fn with_memory_settings(mut self, config: &manifest::ResolvedMemoryFeatureConfig) -> Self {
|
||||
self.memory_settings =
|
||||
config
|
||||
.workspace_settings()
|
||||
@@ -1014,16 +986,18 @@ permission = "write"
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
fn test_config() -> manifest::MemoryConfig {
|
||||
let mut config = manifest::MemoryConfig {
|
||||
extract_threshold: Some(1),
|
||||
..Default::default()
|
||||
};
|
||||
config.bind_workspace_settings(&manifest::WorkspaceMemorySettingsSnapshot {
|
||||
workspace_id: "workspace-1".to_string(),
|
||||
settings_revision: 1,
|
||||
language: "English".to_string(),
|
||||
});
|
||||
fn test_config() -> manifest::ResolvedMemoryFeatureConfig {
|
||||
let mut config = manifest::ResolvedMemoryFeatureConfig::default();
|
||||
config.profile.enabled = true;
|
||||
config.profile.extraction.enabled = true;
|
||||
config.profile.extraction.threshold = Some(1);
|
||||
config
|
||||
.bind_workspace_settings(manifest::WorkspaceMemorySettingsSnapshot {
|
||||
workspace_id: "workspace-1".to_string(),
|
||||
settings_revision: 1,
|
||||
language: "English".to_string(),
|
||||
})
|
||||
.unwrap();
|
||||
config
|
||||
}
|
||||
|
||||
@@ -1282,30 +1256,30 @@ permission = "write"
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn lifecycle_task_requests_backend_consolidation_from_configured_threshold() {
|
||||
async fn lifecycle_task_requests_backend_owned_consolidation_eligibility() {
|
||||
let client = ScriptClient::new(Vec::new());
|
||||
let extension_writes = Arc::new(Mutex::new(Vec::new()));
|
||||
let (event_tx, _) = broadcast::channel(16);
|
||||
let workspace_client = Arc::new(RecordingWorkspaceClient::default());
|
||||
let mut interrupted = capture(2, 250);
|
||||
interrupted.run_exit = CommittedRunExit::Interrupted;
|
||||
let mut task = test_task(
|
||||
let task = test_task(
|
||||
interrupted,
|
||||
Box::new(client),
|
||||
extension_writes,
|
||||
event_tx,
|
||||
workspace_client.clone(),
|
||||
);
|
||||
task.config.consolidation_threshold_files = Some(3);
|
||||
run_background_task(task).await;
|
||||
|
||||
let requests = workspace_client.requests.lock().unwrap();
|
||||
assert!(
|
||||
requests.iter().any(|request| {
|
||||
request.path.contains("memory")
|
||||
&& request.body.as_deref().is_some_and(|body| {
|
||||
body.contains("\"threshold_files\":3") && body.contains("\"force\":false")
|
||||
})
|
||||
&& request
|
||||
.body
|
||||
.as_deref()
|
||||
.is_some_and(|body| body == "{\"force\":false}")
|
||||
}),
|
||||
"recorded requests: {requests:?}"
|
||||
);
|
||||
@@ -1349,17 +1323,6 @@ permission = "write"
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn consolidation_thresholds_enable_backend_request_on_either_limit() {
|
||||
let mut config = manifest::MemoryConfig::default();
|
||||
assert_eq!(consolidation_thresholds(&config), None);
|
||||
config.consolidation_threshold_files = Some(3);
|
||||
assert_eq!(consolidation_thresholds(&config), Some((Some(3), None)));
|
||||
config.consolidation_threshold_files = None;
|
||||
config.consolidation_threshold_bytes = Some(4096);
|
||||
assert_eq!(consolidation_thresholds(&config), Some((None, Some(4096))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn interrupted_parent_run_is_not_extraction_eligible() {
|
||||
assert!(extraction_run_eligible(CommittedRunExit::Finished));
|
||||
@@ -1412,8 +1375,8 @@ permission = "write"
|
||||
#[test]
|
||||
fn threshold_uses_committed_usage_after_pointer() {
|
||||
let capture = capture(2, 250);
|
||||
let mut config = manifest::MemoryConfig::default();
|
||||
config.extract_threshold = Some(1);
|
||||
let mut config = manifest::ResolvedMemoryFeatureConfig::default();
|
||||
config.profile.extraction.threshold = Some(1);
|
||||
assert!(extraction_threshold_reached(
|
||||
&capture,
|
||||
Some(&memory::ExtractPointerPayload {
|
||||
@@ -1500,8 +1463,8 @@ permission = "write"
|
||||
#[test]
|
||||
fn empty_capture_never_schedules_extraction() {
|
||||
let capture = capture(0, 500);
|
||||
let mut config = manifest::MemoryConfig::default();
|
||||
config.extract_threshold = Some(1);
|
||||
let mut config = manifest::ResolvedMemoryFeatureConfig::default();
|
||||
config.profile.extraction.threshold = Some(1);
|
||||
assert!(!extraction_threshold_reached(&capture, None, &config));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -425,6 +425,8 @@ impl Tool for SubWorkerSpawnTool {
|
||||
WorkerManifestConfig::resolution_defaults().merge(child_config),
|
||||
)
|
||||
.map_err(|error| ToolError::ExecutionFailed(format!("resolve child manifest: {error}")))?;
|
||||
bind_child_memory_settings(&self.spawner_manifest, &mut child_manifest)
|
||||
.map_err(ToolError::ExecutionFailed)?;
|
||||
// Delegated children stay bound to their scoped session and cannot use
|
||||
// Workspace attachment tools to replace it with parent-level authority.
|
||||
child_manifest.feature.manage_workdir.enabled = false;
|
||||
@@ -827,6 +829,33 @@ fn profile_error_with_available(error: ProfileError, available: &AvailableProfil
|
||||
)
|
||||
}
|
||||
|
||||
fn bind_child_memory_settings(
|
||||
parent: &manifest::WorkerManifest,
|
||||
child: &mut manifest::WorkerManifest,
|
||||
) -> Result<(), String> {
|
||||
if !child.feature.memory.profile.enabled {
|
||||
return child
|
||||
.feature
|
||||
.memory
|
||||
.validate_execution()
|
||||
.map_err(str::to_string);
|
||||
}
|
||||
let workspace_settings = parent.feature.memory.workspace_settings().ok_or_else(|| {
|
||||
"enabled child Memory feature requires the parent's trusted Workspace settings snapshot"
|
||||
.to_string()
|
||||
})?;
|
||||
child
|
||||
.feature
|
||||
.memory
|
||||
.bind_workspace_settings(workspace_settings)
|
||||
.map_err(str::to_string)?;
|
||||
child
|
||||
.feature
|
||||
.memory
|
||||
.validate_execution()
|
||||
.map_err(str::to_string)
|
||||
}
|
||||
|
||||
fn manifest_to_reusable_config(manifest: &WorkerManifest) -> WorkerManifestConfig {
|
||||
WorkerManifestConfig {
|
||||
worker: WorkerMetaConfig {
|
||||
@@ -894,7 +923,6 @@ fn manifest_to_reusable_config(manifest: &WorkerManifest) -> WorkerManifestConfi
|
||||
model: c.model.clone(),
|
||||
}),
|
||||
web: manifest.web.clone(),
|
||||
memory: manifest.memory.clone(),
|
||||
skills: manifest.skills.clone(),
|
||||
}
|
||||
}
|
||||
@@ -1091,10 +1119,7 @@ enabled = true
|
||||
thread = true
|
||||
|
||||
[feature.memory]
|
||||
enabled = true
|
||||
|
||||
[memory]
|
||||
extract_threshold = 4000
|
||||
enabled = false
|
||||
"#;
|
||||
|
||||
#[tokio::test]
|
||||
@@ -1526,6 +1551,33 @@ extract_threshold = 4000
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn child_memory_inherits_only_the_parents_trusted_settings_snapshot() {
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let mut parent = parent_manifest(temp.path(), None);
|
||||
parent.feature.memory.profile.enabled = true;
|
||||
parent
|
||||
.feature
|
||||
.memory
|
||||
.bind_workspace_settings(manifest::WorkspaceMemorySettingsSnapshot {
|
||||
workspace_id: "workspace-1".to_string(),
|
||||
settings_revision: 4,
|
||||
language: "日本語".to_string(),
|
||||
})
|
||||
.unwrap();
|
||||
let mut child = parent.clone();
|
||||
child.feature.memory.workspace_settings = None;
|
||||
|
||||
bind_child_memory_settings(&parent, &mut child).unwrap();
|
||||
assert_eq!(
|
||||
child.feature.memory.workspace_settings(),
|
||||
parent.feature.memory.workspace_settings()
|
||||
);
|
||||
|
||||
child.feature.memory.profile.enabled = false;
|
||||
assert!(bind_child_memory_settings(&parent, &mut child).is_err());
|
||||
}
|
||||
|
||||
fn write_project_profile_registry(
|
||||
project: &Path,
|
||||
default: Option<&str>,
|
||||
|
||||
+68
-64
@@ -2478,11 +2478,10 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
||||
)
|
||||
});
|
||||
if is_memory_consolidation {
|
||||
let memory_config = self.manifest.memory.as_ref().ok_or_else(|| {
|
||||
WorkerError::InvalidState(
|
||||
"Memory consolidation Worker has no Memory configuration".to_string(),
|
||||
)
|
||||
})?;
|
||||
let memory_config = &self.manifest.feature.memory;
|
||||
memory_config
|
||||
.validate_execution()
|
||||
.map_err(|message| WorkerError::InvalidState(message.to_string()))?;
|
||||
let language = memory_language(memory_config)?;
|
||||
let rendered = self
|
||||
.prompts
|
||||
@@ -2517,11 +2516,8 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
||||
}
|
||||
}
|
||||
let inject_summary = self.inject_resident_summary
|
||||
&& self
|
||||
.manifest
|
||||
.memory
|
||||
.as_ref()
|
||||
.is_some_and(|m| m.inject_summary.unwrap_or(true));
|
||||
&& self.manifest.feature.memory.profile.enabled
|
||||
&& self.manifest.feature.memory.profile.resident.inject_summary;
|
||||
let resident_summary: Option<String> = if inject_summary {
|
||||
match self.resident_summary_from_workspace_authority().await {
|
||||
Ok(summary) => summary,
|
||||
@@ -4552,7 +4548,7 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
||||
}
|
||||
}
|
||||
|
||||
fn memory_language(config: &manifest::MemoryConfig) -> Result<String, WorkerError> {
|
||||
fn memory_language(config: &manifest::ResolvedMemoryFeatureConfig) -> Result<String, WorkerError> {
|
||||
config
|
||||
.workspace_settings()
|
||||
.map(|snapshot| snapshot.language)
|
||||
@@ -5426,7 +5422,8 @@ fn worker_metadata_for_manifest(
|
||||
metadata = metadata.with_workspace_root(local_workspace_root.to_path_buf());
|
||||
}
|
||||
if should_persist_resolved_manifest_snapshot(manifest) {
|
||||
metadata.resolved_manifest_snapshot = serde_json::to_value(manifest).ok();
|
||||
metadata.resolved_manifest_snapshot =
|
||||
manifest::write_persisted_worker_manifest_snapshot(manifest).ok();
|
||||
}
|
||||
metadata
|
||||
}
|
||||
@@ -5439,10 +5436,20 @@ fn validate_workspace_memory_snapshot(
|
||||
let Some(workspace_id) = workspace_context.workspace_id() else {
|
||||
return Ok(());
|
||||
};
|
||||
let snapshot = manifest
|
||||
manifest
|
||||
.feature
|
||||
.memory
|
||||
.as_ref()
|
||||
.and_then(manifest::MemoryConfig::workspace_settings)
|
||||
.validate_execution()
|
||||
.map_err(|message| {
|
||||
WorkerError::InvalidState(format!("Workspace Worker {worker_name}: {message}"))
|
||||
})?;
|
||||
if !manifest.feature.memory.profile.enabled {
|
||||
return Ok(());
|
||||
}
|
||||
let snapshot = manifest
|
||||
.feature
|
||||
.memory
|
||||
.workspace_settings()
|
||||
.ok_or_else(|| {
|
||||
WorkerError::InvalidState(format!(
|
||||
"Workspace Worker {worker_name} has no complete persisted Memory settings snapshot"
|
||||
@@ -5468,11 +5475,7 @@ fn validate_workspace_memory_snapshot(
|
||||
fn should_persist_resolved_manifest_snapshot(manifest: &WorkerManifest) -> bool {
|
||||
manifest.profile.is_some()
|
||||
|| manifest.plugins.has_resolved_plan()
|
||||
|| manifest
|
||||
.memory
|
||||
.as_ref()
|
||||
.and_then(manifest::MemoryConfig::workspace_settings)
|
||||
.is_some()
|
||||
|| manifest.feature.memory.workspace_settings.is_some()
|
||||
}
|
||||
|
||||
fn restore_manifest_from_worker_metadata_snapshot(
|
||||
@@ -5481,12 +5484,14 @@ fn restore_manifest_from_worker_metadata_snapshot(
|
||||
fallback: WorkerManifest,
|
||||
) -> Result<WorkerManifest, WorkerError> {
|
||||
match snapshot {
|
||||
Some(snapshot) => serde_json::from_value(snapshot).map_err(|source| {
|
||||
WorkerError::WorkerMetadataManifestSnapshot {
|
||||
worker_name: worker_name.to_string(),
|
||||
source,
|
||||
}
|
||||
}),
|
||||
Some(snapshot) => {
|
||||
manifest::read_persisted_worker_manifest_snapshot(snapshot).map_err(|source| {
|
||||
WorkerError::WorkerMetadataManifestSnapshot {
|
||||
worker_name: worker_name.to_string(),
|
||||
source,
|
||||
}
|
||||
})
|
||||
}
|
||||
None => Ok(fallback),
|
||||
}
|
||||
}
|
||||
@@ -6198,11 +6203,6 @@ fn prepare_worker_common_with_context_and_model_client(
|
||||
WorkerFilesystemAuthority::Local(LocalWorkingDirectory { root, cwd })
|
||||
}
|
||||
};
|
||||
let mut scope_config = scope_config;
|
||||
if let (Some(mem), Some(local)) = (manifest.memory.as_ref(), filesystem_authority.as_local()) {
|
||||
let layout = memory::WorkspaceLayout::resolve(mem, &local.root);
|
||||
scope_config.deny.extend(memory::deny_write_rules(&layout));
|
||||
}
|
||||
let scope = if scope_config.allow.is_empty() && filesystem_authority.as_local().is_none() {
|
||||
Scope::empty()
|
||||
} else {
|
||||
@@ -6292,8 +6292,7 @@ mod spawned_context_tests {
|
||||
std::fs::create_dir_all(&workspace_root).unwrap();
|
||||
std::fs::create_dir_all(&cwd).unwrap();
|
||||
|
||||
let mut manifest = minimal_manifest_for_context_test(&workspace_root, &cwd);
|
||||
manifest.memory = Some(manifest::MemoryConfig::default());
|
||||
let manifest = minimal_manifest_for_context_test(&workspace_root, &cwd);
|
||||
let common = prepare_worker_common_with_context(
|
||||
&manifest,
|
||||
&PromptCatalogSource::builtins_only(),
|
||||
@@ -6327,8 +6326,7 @@ mod spawned_context_tests {
|
||||
let workspace_root = tmp.path().join("workspace-root");
|
||||
let cwd = workspace_root.join("nested");
|
||||
std::fs::create_dir_all(&cwd).unwrap();
|
||||
let mut manifest = minimal_manifest_for_context_test(&workspace_root, &cwd);
|
||||
manifest.memory = Some(manifest::MemoryConfig::default());
|
||||
let manifest = minimal_manifest_for_context_test(&workspace_root, &cwd);
|
||||
let loader = PromptCatalogSource::builtins_only();
|
||||
let workspace_id = WorkspaceId::new("ws-api-only").unwrap();
|
||||
let common = prepare_worker_common_with_context(
|
||||
@@ -6535,7 +6533,7 @@ permission = "write"
|
||||
|
||||
let restored = restore_manifest_from_worker_metadata_snapshot(
|
||||
"restore-scope",
|
||||
Some(serde_json::to_value(&saved).unwrap()),
|
||||
Some(manifest::write_persisted_worker_manifest_snapshot(&saved).unwrap()),
|
||||
current,
|
||||
)
|
||||
.unwrap();
|
||||
@@ -6590,24 +6588,26 @@ permission = "read"
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
manifest.memory = Some(manifest::MemoryConfig::default());
|
||||
manifest.memory.as_mut().unwrap().bind_workspace_settings(
|
||||
&manifest::WorkspaceMemorySettingsSnapshot {
|
||||
manifest.feature.memory.profile.enabled = true;
|
||||
manifest
|
||||
.feature
|
||||
.memory
|
||||
.bind_workspace_settings(manifest::WorkspaceMemorySettingsSnapshot {
|
||||
workspace_id: "workspace-a".to_string(),
|
||||
settings_revision: 7,
|
||||
language: "Japanese".to_string(),
|
||||
},
|
||||
);
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
let metadata = worker_metadata_for_manifest(&manifest, None, None, None);
|
||||
let restored: WorkerManifest = serde_json::from_value(
|
||||
let restored = manifest::read_persisted_worker_manifest_snapshot(
|
||||
metadata
|
||||
.resolved_manifest_snapshot
|
||||
.expect("Memory settings require a resolved manifest snapshot"),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
restored.memory.unwrap().workspace_settings(),
|
||||
restored.feature.memory.workspace_settings(),
|
||||
Some(manifest::WorkspaceMemorySettingsSnapshot {
|
||||
workspace_id: "workspace-a".to_string(),
|
||||
settings_revision: 7,
|
||||
@@ -6638,7 +6638,7 @@ permission = "read"
|
||||
);
|
||||
|
||||
let mut missing = manifest.clone();
|
||||
missing.memory.as_mut().unwrap().settings_revision = None;
|
||||
missing.feature.memory.workspace_settings = None;
|
||||
assert!(
|
||||
validate_workspace_memory_snapshot(
|
||||
"memory-snapshot",
|
||||
@@ -6715,7 +6715,7 @@ permission = "read"
|
||||
let snapshot = metadata
|
||||
.resolved_manifest_snapshot
|
||||
.expect("plugin-resolved manifest should be snapshotted");
|
||||
let restored: WorkerManifest = serde_json::from_value(snapshot).unwrap();
|
||||
let restored = manifest::read_persisted_worker_manifest_snapshot(snapshot).unwrap();
|
||||
|
||||
assert!(restored.profile.is_none());
|
||||
assert_eq!(restored.plugins.resolved.len(), 1);
|
||||
@@ -8203,13 +8203,16 @@ mod build_summary_prompt_tests {
|
||||
},
|
||||
profile: None,
|
||||
});
|
||||
let mut memory = manifest::MemoryConfig::default();
|
||||
memory.bind_workspace_settings(&manifest::WorkspaceMemorySettingsSnapshot {
|
||||
workspace_id: "workspace-test".to_string(),
|
||||
settings_revision: 3,
|
||||
language: "Japanese".to_string(),
|
||||
});
|
||||
manifest.memory = Some(memory);
|
||||
let mut memory = manifest::ResolvedMemoryFeatureConfig::default();
|
||||
memory.profile.enabled = true;
|
||||
memory
|
||||
.bind_workspace_settings(manifest::WorkspaceMemorySettingsSnapshot {
|
||||
workspace_id: "workspace-test".to_string(),
|
||||
settings_revision: 3,
|
||||
language: "Japanese".to_string(),
|
||||
})
|
||||
.unwrap();
|
||||
manifest.feature.memory = memory;
|
||||
let mut worker = Worker::new(
|
||||
manifest,
|
||||
Engine::<_, Mutable, SessionHistoryMetadata>::new_annotated(NoopClient),
|
||||
@@ -8235,7 +8238,7 @@ mod build_summary_prompt_tests {
|
||||
|
||||
async fn render_system_prompt_with_summary(
|
||||
summary_doc: Option<&str>,
|
||||
memory_config: Option<manifest::MemoryConfig>,
|
||||
memory_config: Option<manifest::ResolvedMemoryFeatureConfig>,
|
||||
resident_injection: bool,
|
||||
) -> String {
|
||||
render_system_prompt_with_resident_sections(
|
||||
@@ -8249,7 +8252,7 @@ mod build_summary_prompt_tests {
|
||||
|
||||
async fn render_system_prompt_with_resident_sections(
|
||||
summary_doc: Option<&str>,
|
||||
memory_config: Option<manifest::MemoryConfig>,
|
||||
memory_config: Option<manifest::ResolvedMemoryFeatureConfig>,
|
||||
gates: ResidentInjectionGates,
|
||||
_unused: bool,
|
||||
) -> String {
|
||||
@@ -8258,12 +8261,15 @@ mod build_summary_prompt_tests {
|
||||
let cwd = dir.path().join("workspace");
|
||||
std::fs::create_dir_all(&cwd).unwrap();
|
||||
let mut manifest = minimal_manifest();
|
||||
manifest.memory = memory_config.clone();
|
||||
manifest.feature.memory = memory_config.clone().unwrap_or_default();
|
||||
if memory_config.is_some() {
|
||||
manifest.feature.memory.profile.enabled = true;
|
||||
}
|
||||
let scope = Scope::writable(&cwd).unwrap();
|
||||
let authority = WorkerFilesystemAuthority::local(cwd.clone(), cwd.clone());
|
||||
let workspace_context = if memory_config
|
||||
.as_ref()
|
||||
.is_some_and(|cfg| cfg.inject_summary.unwrap_or(true))
|
||||
.is_some_and(|cfg| cfg.profile.resident.inject_summary)
|
||||
&& gates.summary
|
||||
{
|
||||
stub_memory_backend_context(summary_doc.and_then(summary_content_for_backend))
|
||||
@@ -8350,7 +8356,7 @@ mod build_summary_prompt_tests {
|
||||
async fn resident_summary_body_is_injected_without_frontmatter() {
|
||||
let rendered = render_system_prompt_with_summary(
|
||||
Some(&summary_doc("summary body for resident prompt\n")),
|
||||
Some(manifest::MemoryConfig::default()),
|
||||
Some(manifest::ResolvedMemoryFeatureConfig::default()),
|
||||
true,
|
||||
)
|
||||
.await;
|
||||
@@ -8362,10 +8368,8 @@ mod build_summary_prompt_tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn resident_summary_injection_can_be_disabled_by_manifest() {
|
||||
let memory = manifest::MemoryConfig {
|
||||
inject_summary: Some(false),
|
||||
..manifest::MemoryConfig::default()
|
||||
};
|
||||
let mut memory = manifest::ResolvedMemoryFeatureConfig::default();
|
||||
memory.profile.resident.inject_summary = false;
|
||||
let rendered = render_system_prompt_with_summary(
|
||||
Some(&summary_doc("disabled summary body\n")),
|
||||
Some(memory),
|
||||
@@ -8377,7 +8381,7 @@ mod build_summary_prompt_tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resident_summary_is_absent_without_memory_config() {
|
||||
async fn resident_summary_is_absent_when_memory_feature_is_disabled() {
|
||||
let rendered = render_system_prompt_with_summary(
|
||||
Some(&summary_doc("memory-disabled summary body\n")),
|
||||
None,
|
||||
@@ -8392,7 +8396,7 @@ mod build_summary_prompt_tests {
|
||||
async fn malformed_resident_summary_does_not_fail_render() {
|
||||
let rendered = render_system_prompt_with_summary(
|
||||
Some("---\nthis is not yaml: : :\n---\nbad summary body\n"),
|
||||
Some(manifest::MemoryConfig::default()),
|
||||
Some(manifest::ResolvedMemoryFeatureConfig::default()),
|
||||
true,
|
||||
)
|
||||
.await;
|
||||
@@ -8405,7 +8409,7 @@ mod build_summary_prompt_tests {
|
||||
async fn resident_summary_gate_false_omits_only_summary() {
|
||||
let prompt = render_system_prompt_with_resident_sections(
|
||||
Some(&summary_doc("resident summary marker")),
|
||||
Some(manifest::MemoryConfig::default()),
|
||||
Some(manifest::ResolvedMemoryFeatureConfig::default()),
|
||||
ResidentInjectionGates { summary: false },
|
||||
true,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user