fix: gate workspace memory lifecycle authority
This commit is contained in:
@@ -904,6 +904,7 @@ pub(crate) fn wire_event_bridges_on_engine<C, St>(
|
|||||||
fn add_memory_lifecycle_if_configured<M>(
|
fn add_memory_lifecycle_if_configured<M>(
|
||||||
registry: &mut FeatureRegistryBuilder,
|
registry: &mut FeatureRegistryBuilder,
|
||||||
config: Option<manifest::MemoryConfig>,
|
config: Option<manifest::MemoryConfig>,
|
||||||
|
workspace_bound: bool,
|
||||||
build: impl FnOnce(manifest::MemoryConfig) -> std::io::Result<M>,
|
build: impl FnOnce(manifest::MemoryConfig) -> std::io::Result<M>,
|
||||||
) -> std::io::Result<bool>
|
) -> std::io::Result<bool>
|
||||||
where
|
where
|
||||||
@@ -912,6 +913,15 @@ where
|
|||||||
let Some(config) = config else {
|
let Some(config) = config else {
|
||||||
return Ok(false);
|
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",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
return Ok(false);
|
||||||
|
}
|
||||||
registry.add_module(build(config)?);
|
registry.add_module(build(config)?);
|
||||||
Ok(true)
|
Ok(true)
|
||||||
}
|
}
|
||||||
@@ -1009,7 +1019,14 @@ where
|
|||||||
let worker_enabled = feature_config.worker.enabled;
|
let worker_enabled = feature_config.worker.enabled;
|
||||||
let sub_worker_enabled = feature_config.sub_worker.enabled;
|
let sub_worker_enabled = feature_config.sub_worker.enabled;
|
||||||
let mut feature_registry = FeatureRegistryBuilder::new();
|
let mut feature_registry = FeatureRegistryBuilder::new();
|
||||||
add_memory_lifecycle_if_configured(&mut feature_registry, memory_config.clone(), |config| {
|
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(),
|
||||||
|
|config| {
|
||||||
let workspace_client = worker.workspace_client_handle();
|
let workspace_client = worker.workspace_client_handle();
|
||||||
if !workspace_client.is_available() || workspace_client.workspace_id().is_none() {
|
if !workspace_client.is_available() || workspace_client.workspace_id().is_none() {
|
||||||
return Err(std::io::Error::new(
|
return Err(std::io::Error::new(
|
||||||
@@ -1030,7 +1047,8 @@ where
|
|||||||
worker.working_event_sender(),
|
worker.working_event_sender(),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
})?;
|
},
|
||||||
|
)?;
|
||||||
if sub_worker_enabled && !worker_enabled {
|
if sub_worker_enabled && !worker_enabled {
|
||||||
feature_registry.add_module(
|
feature_registry.add_module(
|
||||||
crate::feature::builtin::manage_worker::sub_worker_control_feature(
|
crate::feature::builtin::manage_worker::sub_worker_control_feature(
|
||||||
@@ -2151,7 +2169,7 @@ mod tests {
|
|||||||
use tokio::net::UnixListener;
|
use tokio::net::UnixListener;
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn memory_lifecycle_registration_depends_only_on_memory_config_presence() {
|
fn memory_lifecycle_registration_requires_bound_workspace_memory_config() {
|
||||||
#[derive(Clone)]
|
#[derive(Clone)]
|
||||||
struct TestMemoryLifecycleModule;
|
struct TestMemoryLifecycleModule;
|
||||||
|
|
||||||
@@ -2173,14 +2191,17 @@ mod tests {
|
|||||||
|
|
||||||
let mut registry = FeatureRegistryBuilder::new();
|
let mut registry = FeatureRegistryBuilder::new();
|
||||||
let configured = std::cell::Cell::new(false);
|
let configured = std::cell::Cell::new(false);
|
||||||
let installed = add_memory_lifecycle_if_configured(
|
let mut memory_config = manifest::MemoryConfig::default();
|
||||||
&mut registry,
|
memory_config.bind_workspace_settings(&manifest::WorkspaceMemorySettingsSnapshot {
|
||||||
Some(manifest::MemoryConfig::default()),
|
workspace_id: "workspace-1".to_string(),
|
||||||
|_| {
|
settings_revision: 1,
|
||||||
|
language: "English".to_string(),
|
||||||
|
});
|
||||||
|
let installed =
|
||||||
|
add_memory_lifecycle_if_configured(&mut registry, Some(memory_config), true, |_| {
|
||||||
configured.set(true);
|
configured.set(true);
|
||||||
Ok(TestMemoryLifecycleModule)
|
Ok(TestMemoryLifecycleModule)
|
||||||
},
|
})
|
||||||
)
|
|
||||||
.unwrap();
|
.unwrap();
|
||||||
assert!(installed);
|
assert!(installed);
|
||||||
assert!(configured.get());
|
assert!(configured.get());
|
||||||
@@ -2189,10 +2210,33 @@ mod tests {
|
|||||||
let installed = add_memory_lifecycle_if_configured::<TestMemoryLifecycleModule>(
|
let installed = add_memory_lifecycle_if_configured::<TestMemoryLifecycleModule>(
|
||||||
&mut registry,
|
&mut registry,
|
||||||
None,
|
None,
|
||||||
|
false,
|
||||||
|_| panic!("disabled Memory must not construct its lifecycle Feature"),
|
|_| panic!("disabled Memory must not construct its lifecycle Feature"),
|
||||||
)
|
)
|
||||||
.unwrap();
|
.unwrap();
|
||||||
assert!(!installed);
|
assert!(!installed);
|
||||||
|
|
||||||
|
let installed = add_memory_lifecycle_if_configured::<TestMemoryLifecycleModule>(
|
||||||
|
&mut registry,
|
||||||
|
Some(manifest::MemoryConfig::default()),
|
||||||
|
false,
|
||||||
|
|_| panic!("Memory without a Backend-authored settings snapshot must stay disabled"),
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
assert!(!installed);
|
||||||
|
|
||||||
|
let error = add_memory_lifecycle_if_configured::<TestMemoryLifecycleModule>(
|
||||||
|
&mut registry,
|
||||||
|
Some(manifest::MemoryConfig::default()),
|
||||||
|
true,
|
||||||
|
|_| panic!("invalid Workspace Memory config must fail before Feature construction"),
|
||||||
|
)
|
||||||
|
.unwrap_err();
|
||||||
|
assert!(
|
||||||
|
error
|
||||||
|
.to_string()
|
||||||
|
.contains("Backend-authored settings snapshot")
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -164,6 +164,7 @@ where
|
|||||||
identity: identity.clone(),
|
identity: identity.clone(),
|
||||||
history_entries: 0,
|
history_entries: 0,
|
||||||
})?;
|
})?;
|
||||||
|
worker.disable_manifest_lifecycle_features();
|
||||||
if let Some(session) = inherited_workdir_session {
|
if let Some(session) = inherited_workdir_session {
|
||||||
worker.bind_workdir_session(Some(session));
|
worker.bind_workdir_session(Some(session));
|
||||||
}
|
}
|
||||||
@@ -578,6 +579,7 @@ pub(crate) async fn spawn_internal_worker_session(
|
|||||||
.map_err(|source| InternalWorkerSessionError::Build {
|
.map_err(|source| InternalWorkerSessionError::Build {
|
||||||
message: source.to_string(),
|
message: source.to_string(),
|
||||||
})?;
|
})?;
|
||||||
|
worker.disable_manifest_lifecycle_features();
|
||||||
if let Some(session) = inherited_workdir_session {
|
if let Some(session) = inherited_workdir_session {
|
||||||
worker.bind_workdir_session(Some(session));
|
worker.bind_workdir_session(Some(session));
|
||||||
}
|
}
|
||||||
@@ -671,6 +673,7 @@ pub(crate) fn prepare_internal_worker_from_spec(
|
|||||||
.map_err(|source| InternalWorkerSessionError::Build {
|
.map_err(|source| InternalWorkerSessionError::Build {
|
||||||
message: source.to_string(),
|
message: source.to_string(),
|
||||||
})?;
|
})?;
|
||||||
|
worker.disable_manifest_lifecycle_features();
|
||||||
if let Some(session) = inherited_workdir_session {
|
if let Some(session) = inherited_workdir_session {
|
||||||
worker.bind_workdir_session(Some(session));
|
worker.bind_workdir_session(Some(session));
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1130,6 +1130,9 @@ pub struct Worker<C: LlmClient, St: Store> {
|
|||||||
hook_registry: Option<Arc<HookRegistry>>,
|
hook_registry: Option<Arc<HookRegistry>>,
|
||||||
/// Executable background tasks registered by successfully installed features.
|
/// Executable background tasks registered by successfully installed features.
|
||||||
feature_background_tasks: FeatureBackgroundTaskRegistry,
|
feature_background_tasks: FeatureBackgroundTaskRegistry,
|
||||||
|
/// Internal Workers install an explicit Feature composition and disable
|
||||||
|
/// manifest-derived lifecycle Features before controller startup.
|
||||||
|
manifest_lifecycle_features_enabled: bool,
|
||||||
interceptor_installed: bool,
|
interceptor_installed: bool,
|
||||||
/// Shared compaction state (present when threshold is configured).
|
/// Shared compaction state (present when threshold is configured).
|
||||||
compact_state: Option<Arc<CompactState>>,
|
compact_state: Option<Arc<CompactState>>,
|
||||||
@@ -1423,6 +1426,7 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
|||||||
hook_builder: HookRegistryBuilder::new(),
|
hook_builder: HookRegistryBuilder::new(),
|
||||||
hook_registry: None,
|
hook_registry: None,
|
||||||
feature_background_tasks: FeatureBackgroundTaskRegistry::default(),
|
feature_background_tasks: FeatureBackgroundTaskRegistry::default(),
|
||||||
|
manifest_lifecycle_features_enabled: true,
|
||||||
interceptor_installed: false,
|
interceptor_installed: false,
|
||||||
compact_state: None,
|
compact_state: None,
|
||||||
usage_tracker: Arc::new(UsageTracker::new()),
|
usage_tracker: Arc::new(UsageTracker::new()),
|
||||||
@@ -1800,6 +1804,14 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
|||||||
self.session.replace_history(entries);
|
self.session.replace_history(entries);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(crate) fn disable_manifest_lifecycle_features(&mut self) {
|
||||||
|
self.manifest_lifecycle_features_enabled = false;
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn manifest_lifecycle_features_enabled(&self) -> bool {
|
||||||
|
self.manifest_lifecycle_features_enabled
|
||||||
|
}
|
||||||
|
|
||||||
/// Install enabled feature modules into the Worker host surfaces.
|
/// Install enabled feature modules into the Worker host surfaces.
|
||||||
pub fn install_features(
|
pub fn install_features(
|
||||||
&mut self,
|
&mut self,
|
||||||
@@ -4676,6 +4688,7 @@ where
|
|||||||
hook_builder: HookRegistryBuilder::new(),
|
hook_builder: HookRegistryBuilder::new(),
|
||||||
hook_registry: None,
|
hook_registry: None,
|
||||||
feature_background_tasks: FeatureBackgroundTaskRegistry::default(),
|
feature_background_tasks: FeatureBackgroundTaskRegistry::default(),
|
||||||
|
manifest_lifecycle_features_enabled: true,
|
||||||
interceptor_installed: false,
|
interceptor_installed: false,
|
||||||
compact_state: None,
|
compact_state: None,
|
||||||
usage_tracker: Arc::new(UsageTracker::new()),
|
usage_tracker: Arc::new(UsageTracker::new()),
|
||||||
@@ -4757,6 +4770,7 @@ where
|
|||||||
hook_builder: HookRegistryBuilder::new(),
|
hook_builder: HookRegistryBuilder::new(),
|
||||||
hook_registry: None,
|
hook_registry: None,
|
||||||
feature_background_tasks: FeatureBackgroundTaskRegistry::default(),
|
feature_background_tasks: FeatureBackgroundTaskRegistry::default(),
|
||||||
|
manifest_lifecycle_features_enabled: false,
|
||||||
interceptor_installed: false,
|
interceptor_installed: false,
|
||||||
compact_state: None,
|
compact_state: None,
|
||||||
usage_tracker: Arc::new(UsageTracker::new()),
|
usage_tracker: Arc::new(UsageTracker::new()),
|
||||||
@@ -4873,6 +4887,7 @@ where
|
|||||||
hook_builder: HookRegistryBuilder::new(),
|
hook_builder: HookRegistryBuilder::new(),
|
||||||
hook_registry: None,
|
hook_registry: None,
|
||||||
feature_background_tasks: FeatureBackgroundTaskRegistry::default(),
|
feature_background_tasks: FeatureBackgroundTaskRegistry::default(),
|
||||||
|
manifest_lifecycle_features_enabled: true,
|
||||||
interceptor_installed: false,
|
interceptor_installed: false,
|
||||||
compact_state: None,
|
compact_state: None,
|
||||||
usage_tracker: Arc::new(UsageTracker::new()),
|
usage_tracker: Arc::new(UsageTracker::new()),
|
||||||
@@ -5243,6 +5258,7 @@ where
|
|||||||
hook_builder: HookRegistryBuilder::new(),
|
hook_builder: HookRegistryBuilder::new(),
|
||||||
hook_registry: None,
|
hook_registry: None,
|
||||||
feature_background_tasks: FeatureBackgroundTaskRegistry::default(),
|
feature_background_tasks: FeatureBackgroundTaskRegistry::default(),
|
||||||
|
manifest_lifecycle_features_enabled: true,
|
||||||
interceptor_installed: false,
|
interceptor_installed: false,
|
||||||
compact_state: None,
|
compact_state: None,
|
||||||
usage_tracker: Arc::new(UsageTracker::new()),
|
usage_tracker: Arc::new(UsageTracker::new()),
|
||||||
|
|||||||
Reference in New Issue
Block a user