Skip to content

Commit 4cfba02

Browse files
committed
fix: apply PR #208 review comments for lifecycle and configuration issues
1 parent 45763bb commit 4cfba02

6 files changed

Lines changed: 62 additions & 72 deletions

File tree

.clippy.toml

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Optimized clippy configuration for faster CI builds
2-
msrv = "1.70.0"
2+
msrv = "1.82.0"
33
avoid-breaking-exported-api = false
44
allow-mixed-uninlined-format-args = true
55
allow-unwrap-in-tests = true
@@ -8,7 +8,6 @@ allow-dbg-in-tests = true
88
too-many-arguments-threshold = 12
99
type-complexity-threshold = 250
1010
cognitive-complexity-threshold = 30
11-
unreadable-literal-threshold = 100
1211
# Reduce false positives and focus on important issues
1312
disallowed-names = []
1413
doc-valid-idents = ["Cli", "TODO", "FIXME", "XXX", "WIP", "WARN", "NOTE", "INFO", "DEBUG"]
@@ -17,12 +16,4 @@ max-trait-bounds = 3
1716
max-fn-params-bools = 3
1817
enum-variant-name-threshold = 50
1918
# Allow certain patterns for faster development
20-
single-char-binding-names-threshold = 4
21-
# Skip expensive lints in CI
22-
cargo-ignore = [
23-
"clippy::nursery/",
24-
"clippy::pedantic/",
25-
"clippy::cargo/",
26-
"clippy::complexity/",
27-
"clippy::perf/",
28-
]
19+
single-char-binding-names-threshold = 4

crates/terraphim_agent_application/src/application.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ pub struct TerraphimAgentApplication {
4747
state: Arc<RwLock<ApplicationState>>,
4848
/// Configuration manager
4949
config_manager: Arc<ConfigurationManager>,
50-
/// Lifecycle manager
51-
lifecycle_manager: Arc<LifecycleManager>,
50+
/// Lifecycle manager
51+
lifecycle_manager: Arc<RwLock<LifecycleManager>>,
5252
/// Deployment manager
5353
deployment_manager: Arc<DeploymentManager>,
5454
/// Hot reload manager
@@ -262,7 +262,7 @@ impl TerraphimAgentApplication {
262262
let config_manager = Arc::new(ConfigurationManager::new(config_path).await?);
263263
let config = config_manager.get_config().await;
264264

265-
let lifecycle_manager = Arc::new(LifecycleManager::new(config.clone()).await?);
265+
let lifecycle_manager = LifecycleManager::new(config.clone()).await?;
266266
let deployment_manager = Arc::new(DeploymentManager::new(config.clone()).await?);
267267
let hot_reload_manager = Arc::new(HotReloadManager::new(config.clone()).await?);
268268
let diagnostics_manager = Arc::new(DiagnosticsManager::new(config.clone()).await?);
@@ -272,7 +272,7 @@ impl TerraphimAgentApplication {
272272
Ok(Self {
273273
state: Arc::new(RwLock::new(ApplicationState::default())),
274274
config_manager,
275-
lifecycle_manager,
275+
lifecycle_manager: Arc::new(RwLock::new(lifecycle_manager)),
276276
deployment_manager,
277277
hot_reload_manager,
278278
diagnostics_manager,
@@ -483,7 +483,7 @@ impl Application for TerraphimAgentApplication {
483483
self.start_message_handler().await?;
484484

485485
// Start lifecycle manager
486-
self.lifecycle_manager.start().await?;
486+
self.lifecycle_manager.write().await.start().await?;
487487

488488
// Start deployment manager
489489
self.deployment_manager.start().await?;
@@ -568,7 +568,7 @@ impl Application for TerraphimAgentApplication {
568568
warn!("Error stopping deployment manager: {}", e);
569569
}
570570

571-
if let Err(e) = self.lifecycle_manager.stop().await {
571+
if let Err(e) = self.lifecycle_manager.write().await.stop().await {
572572
warn!("Error stopping lifecycle manager: {}", e);
573573
}
574574

crates/terraphim_agent_application/src/config.rs

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -262,19 +262,19 @@ impl Default for ApplicationConfig {
262262
}
263263
}
264264

265-
/// Configuration manager with hot reloading capabilities
266-
pub struct ConfigurationManager {
267-
/// Current configuration
268-
config: Arc<RwLock<ApplicationConfig>>,
269-
/// Configuration file path
270-
config_path: PathBuf,
271-
/// File watcher for hot reloading
272-
_watcher: Option<RecommendedWatcher>,
273-
/// Configuration change notifications
274-
change_tx: mpsc::UnboundedSender<ConfigurationChange>,
275-
/// Configuration change receiver
276-
change_rx: Arc<RwLock<Option<mpsc::UnboundedReceiver<ConfigurationChange>>>>,
277-
}
265+
/// Configuration manager with hot reloading capabilities
266+
pub struct ConfigurationManager {
267+
/// Current configuration
268+
config: Arc<RwLock<ApplicationConfig>>,
269+
/// Configuration file path
270+
config_path: PathBuf,
271+
/// File watcher for hot reloading
272+
_watcher: Option<RecommendedWatcher>,
273+
/// Configuration change notifications
274+
change_tx: mpsc::UnboundedSender<ConfigurationChange>,
275+
/// Configuration change receiver
276+
change_rx: Arc<RwLock<Option<mpsc::UnboundedReceiver<ConfigurationChange>>>>,
277+
}
278278

279279
/// Configuration change notification
280280
#[derive(Debug, Clone)]
@@ -322,21 +322,20 @@ impl ConfigurationManager {
322322
})
323323
}
324324

325-
/// Load configuration from file
326-
async fn load_config(config_path: &Path) -> ApplicationResult<ApplicationConfig> {
327-
let mut config_builder = Config::builder()
328-
.add_source(File::from(config_path).required(false))
329-
.add_source(Environment::with_prefix("TERRAPHIM"));
330-
331-
// Add default configuration
332-
let default_config = ApplicationConfig::default();
333-
let default_toml = toml::to_string(&default_config)
334-
.map_err(|e| ApplicationError::ConfigurationError(e.to_string()))?;
335-
336-
config_builder = config_builder.add_source(config::File::from_str(
337-
&default_toml,
338-
config::FileFormat::Toml,
339-
));
325+
/// Load configuration from file
326+
async fn load_config(config_path: &Path) -> ApplicationResult<ApplicationConfig> {
327+
// Add default configuration first (base layer)
328+
let default_config = ApplicationConfig::default();
329+
let default_toml = toml::to_string(&default_config)
330+
.map_err(|e| ApplicationError::ConfigurationError(e.to_string()))?;
331+
332+
let config_builder = Config::builder()
333+
.add_source(config::File::from_str(
334+
&default_toml,
335+
config::FileFormat::Toml,
336+
))
337+
.add_source(File::from(config_path).required(false))
338+
.add_source(Environment::with_prefix("TERRAPHIM"));
340339

341340
let config = config_builder
342341
.build()

crates/terraphim_agent_application/src/hot_reload.rs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ pub struct ComponentSpec {
6363
pub config: serde_json::Value,
6464
}
6565

66-
/// Types of components that can be hot reloaded
67-
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
68-
pub enum ComponentType {
66+
/// Types of components that can be hot reloaded
67+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash)]
68+
pub enum ComponentType {
6969
/// Agent behavior
7070
AgentBehavior,
7171
/// Configuration
@@ -223,15 +223,13 @@ impl HotReloadManager {
223223
component_name, component.component_type
224224
);
225225

226-
// Reload dependencies first
227-
for dependency in &component.dependencies {
228-
if let Err(e) = self
229-
.perform_reload(dependency, ReloadType::Dependency)
230-
.await
231-
{
232-
warn!("Failed to reload dependency {}: {}", dependency, e);
233-
}
234-
}
226+
// Reload dependencies first
227+
for dependency in &component.dependencies {
228+
if let Err(e) = Box::pin(self.perform_reload(dependency, ReloadType::Dependency)).await
229+
{
230+
warn!("Failed to reload dependency {}: {}", dependency, e);
231+
}
232+
}
235233

236234
// Perform the actual reload based on component type and strategy
237235
match self.reload_component_impl(&component).await {

crates/terraphim_agent_application/src/lifecycle.rs

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ use crate::{ApplicationConfig, ApplicationError, ApplicationResult};
1313
#[async_trait]
1414
pub trait LifecycleManagement: Send + Sync {
1515
/// Start lifecycle management
16-
async fn start(&self) -> ApplicationResult<()>;
16+
async fn start(&mut self) -> ApplicationResult<()>;
1717

1818
/// Stop lifecycle management
19-
async fn stop(&self) -> ApplicationResult<()>;
19+
async fn stop(&mut self) -> ApplicationResult<()>;
2020

2121
/// Perform health check
2222
async fn health_check(&self) -> ApplicationResult<bool>;
@@ -56,19 +56,21 @@ impl LifecycleManager {
5656
}
5757
}
5858

59-
#[async_trait]
60-
impl LifecycleManagement for LifecycleManager {
61-
async fn start(&self) -> ApplicationResult<()> {
62-
info!("Starting lifecycle manager");
63-
// In a real implementation, this would initialize lifecycle components
64-
Ok(())
65-
}
59+
#[async_trait]
60+
impl LifecycleManagement for LifecycleManager {
61+
async fn start(&mut self) -> ApplicationResult<()> {
62+
info!("Starting lifecycle manager");
63+
self.start_time = Some(SystemTime::now());
64+
// In a real implementation, this would initialize lifecycle components
65+
Ok(())
66+
}
6667

67-
async fn stop(&self) -> ApplicationResult<()> {
68-
info!("Stopping lifecycle manager");
69-
// In a real implementation, this would cleanup lifecycle components
70-
Ok(())
71-
}
68+
async fn stop(&mut self) -> ApplicationResult<()> {
69+
info!("Stopping lifecycle manager");
70+
self.start_time = None;
71+
// In a real implementation, this would cleanup lifecycle components
72+
Ok(())
73+
}
7274

7375
async fn health_check(&self) -> ApplicationResult<bool> {
7476
debug!("Lifecycle manager health check");

crates/terraphim_kg_orchestration/src/supervision.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ impl SupervisionTreeOrchestrator {
524524
}
525525

526526
/// Shutdown the orchestrator
527-
async fn shutdown(&self) -> OrchestrationResult<()> {
527+
pub async fn shutdown(&self) -> OrchestrationResult<()> {
528528
info!("Shutting down supervision tree orchestrator");
529529

530530
// Terminate all active workflows

0 commit comments

Comments
 (0)