Skip to content

Commit e082787

Browse files
refactor: cleaning up unused traits and their methods (#14)
* fix: update SSE URL to use localhost and adjust server readiness poll timeout * refactor: implement exponential backoff for server readiness polling * refactor: remove DynRunnerProcessManager and clean up unused traits and simplify process manager interfaces * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 09ddf8e commit e082787

File tree

12 files changed

+31
-675
lines changed

12 files changed

+31
-675
lines changed

crates/gpmcp-layer-core/src/layer.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::config::{RetryConfig, RunnerConfig};
22
use crate::error::GpmcpError;
3-
use crate::process_manager_trait::DynRunnerProcessManager;
3+
use crate::process_manager_trait::RunnerProcessManager;
44
use crate::runner::inner::GpmcpRunnerInner;
55
use backon::{ExponentialBuilder, Retryable};
66
use std::future::Future;
@@ -13,17 +13,14 @@ pub struct Initialized;
1313
pub struct Uninitialized;
1414

1515
#[derive(Clone)]
16-
pub struct GpmcpLayer<Status> {
16+
pub struct GpmcpLayer<Status, Manager> {
1717
runner_config: RunnerConfig,
18-
process_manager: Arc<dyn DynRunnerProcessManager>,
19-
inner: Arc<Mutex<GpmcpRunnerInner<Status>>>,
18+
process_manager: Arc<Manager>,
19+
inner: Arc<Mutex<GpmcpRunnerInner<Status, Manager>>>,
2020
retry_config: ExponentialBuilder,
2121
}
22-
impl GpmcpLayer<Uninitialized> {
23-
pub fn new(
24-
runner_config: RunnerConfig,
25-
process_manager: Arc<dyn DynRunnerProcessManager>,
26-
) -> Self {
22+
impl<Manager: RunnerProcessManager> GpmcpLayer<Uninitialized, Manager> {
23+
pub fn new(runner_config: RunnerConfig, process_manager: Arc<Manager>) -> Self {
2724
Self {
2825
inner: Arc::new(Mutex::new(GpmcpRunnerInner::new(
2926
runner_config.clone(),
@@ -34,7 +31,7 @@ impl GpmcpLayer<Uninitialized> {
3431
process_manager,
3532
}
3633
}
37-
pub async fn connect(self) -> Result<GpmcpLayer<Initialized>, GpmcpError> {
34+
pub async fn connect(self) -> Result<GpmcpLayer<Initialized, Manager>, GpmcpError> {
3835
let initialized_inner = self.inner.lock().await.connect().await?;
3936
Ok(GpmcpLayer {
4037
runner_config: self.runner_config,
@@ -57,7 +54,7 @@ impl GpmcpLayer<Uninitialized> {
5754
retry_builder
5855
}
5956
}
60-
impl GpmcpLayer<Initialized> {
57+
impl<Manager: RunnerProcessManager> GpmcpLayer<Initialized, Manager> {
6158
/// Generic retry mechanism for operations that may fail due to connection issues
6259
/// Uses backon library with GpmcpError.is_retryable() to determine if an error should be retried
6360
async fn attempt_with_retry<T, F, Fut>(&self, operation: F) -> Result<T, GpmcpError>

crates/gpmcp-layer-core/src/process.rs

Lines changed: 0 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use anyhow::Result;
22
use async_trait::async_trait;
33
use std::collections::HashMap;
4-
use std::time::Duration;
54

65
/// Unique identifier for a process
76
pub type ProcessId = u32;
@@ -82,85 +81,16 @@ pub trait ProcessLifecycle: Send + Sync {
8281
working_dir: Option<&str>,
8382
env: &HashMap<String, String>,
8483
) -> Result<Self::Handle>;
85-
86-
/// Check if a process is still running and healthy
87-
async fn is_process_healthy(&self, handle: &dyn ProcessHandle) -> bool;
88-
89-
/// Get detailed information about a process
90-
async fn get_process_info(&self, handle: &dyn ProcessHandle) -> Result<ProcessInfo>;
91-
92-
/// Wait for a process to exit with optional timeout
93-
async fn wait_for_exit(
94-
&self,
95-
handle: &mut dyn ProcessHandle,
96-
timeout: Option<Duration>,
97-
) -> Result<ProcessStatus>;
9884
}
9985

10086
/// Trait for comprehensive process termination including process trees
10187
#[async_trait]
10288
pub trait ProcessTermination: Send + Sync {
103-
/// Terminate a single process gracefully (SIGTERM on Unix)
104-
async fn terminate_gracefully(&self, handle: &mut dyn ProcessHandle) -> TerminationResult;
105-
106-
/// Force kill a single process (SIGKILL on Unix)
107-
async fn force_kill(&self, handle: &mut dyn ProcessHandle) -> TerminationResult;
108-
10989
/// Find all child processes of a given process
11090
async fn find_child_processes(&self, pid: ProcessId) -> Result<Vec<ProcessId>>;
11191

11292
/// Terminate an entire process tree (parent and all descendants)
11393
async fn terminate_process_tree(&self, root_pid: ProcessId) -> TerminationResult;
114-
115-
/// Terminate a process group (Unix only, returns ProcessNotFound on Windows)
116-
async fn terminate_process_group(&self, pid: ProcessId) -> TerminationResult;
117-
118-
/// Complete termination strategy: process group -> process tree -> individual process
119-
async fn terminate_completely(&self, handle: &mut dyn ProcessHandle) -> TerminationResult {
120-
if let Some(pid) = handle.get_pid() {
121-
// Step 1: Try process group termination (Unix only)
122-
match self.terminate_process_group(pid).await {
123-
TerminationResult::Success => return TerminationResult::Success,
124-
TerminationResult::ProcessNotFound => {
125-
// Process group not found, continue to next step
126-
}
127-
_ => {
128-
// Process group termination failed, try process tree
129-
}
130-
}
131-
132-
// Step 2: Try process tree termination
133-
match self.terminate_process_tree(pid).await {
134-
TerminationResult::Success => return TerminationResult::Success,
135-
TerminationResult::ProcessNotFound => {
136-
// Process tree not found, continue to individual termination
137-
}
138-
_ => {
139-
// Process tree termination failed, continue to individual termination
140-
}
141-
}
142-
}
143-
144-
// Step 3: Individual process termination with escalation
145-
match self.terminate_gracefully(handle).await {
146-
TerminationResult::Success => {
147-
// Wait a bit to see if process exits gracefully
148-
tokio::time::sleep(Duration::from_millis(1000)).await;
149-
150-
// If still running, force kill
151-
if handle.is_running().await {
152-
self.force_kill(handle).await
153-
} else {
154-
TerminationResult::Success
155-
}
156-
}
157-
TerminationResult::ProcessNotFound => TerminationResult::Success,
158-
_ => {
159-
// Graceful termination failed, try force kill
160-
self.force_kill(handle).await
161-
}
162-
}
163-
}
16494
}
16595

16696
/// Trait representing a handle to a running process
@@ -172,18 +102,6 @@ pub trait ProcessHandle: Send + Sync {
172102
/// Get the command that started this process
173103
fn get_command(&self) -> &str;
174104

175-
/// Get the arguments passed to this process
176-
fn get_args(&self) -> &[String];
177-
178-
/// Check if the process is still running (non-blocking)
179-
async fn is_running(&self) -> bool;
180-
181-
/// Try to get exit status without blocking
182-
async fn try_wait(&mut self) -> Result<Option<ProcessStatus>>;
183-
184-
/// Wait for the process to exit (blocking)
185-
async fn wait(&mut self) -> Result<ProcessStatus>;
186-
187105
/// Kill the process (platform-specific implementation)
188106
async fn kill(&mut self) -> Result<()>;
189107
}
@@ -207,9 +125,6 @@ pub trait ProcessManagerFactory {
207125

208126
/// Create a process manager for the current platform
209127
fn create_process_manager() -> Self::Manager;
210-
211-
/// Get the platform name for logging and debugging
212-
fn platform_name() -> &'static str;
213128
}
214129

215130
/// Implementation of ProcessHandle for boxed trait objects to enable associated type usage
@@ -223,22 +138,6 @@ impl ProcessHandle for Box<dyn ProcessHandle> {
223138
(**self).get_command()
224139
}
225140

226-
fn get_args(&self) -> &[String] {
227-
(**self).get_args()
228-
}
229-
230-
async fn is_running(&self) -> bool {
231-
(**self).is_running().await
232-
}
233-
234-
async fn try_wait(&mut self) -> Result<Option<ProcessStatus>> {
235-
(**self).try_wait().await
236-
}
237-
238-
async fn wait(&mut self) -> Result<ProcessStatus> {
239-
(**self).wait().await
240-
}
241-
242141
async fn kill(&mut self) -> Result<()> {
243142
(**self).kill().await
244143
}
Lines changed: 2 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
use anyhow::Result;
22
use async_trait::async_trait;
3-
use std::collections::HashMap;
43

54
use crate::config::RunnerConfig;
6-
use crate::process::{ProcessHandle, ProcessId};
5+
use crate::process::ProcessHandle;
76

87
/// High-level process manager trait for platform-independent process orchestration
98
///
@@ -93,41 +92,6 @@ pub trait RunnerProcessManager: Send + Sync {
9392
/// ```
9493
async fn start_server(&self) -> Result<Self::Handle>;
9594

96-
/// Spawn a new process with the given parameters and track it for cleanup
97-
///
98-
/// This method spawns a process with the specified command and arguments, optionally
99-
/// setting a working directory and environment variables. The spawned process will
100-
/// be automatically tracked and included in cleanup operations.
101-
///
102-
/// # Arguments
103-
///
104-
/// * `command` - The command to execute
105-
/// * `args` - Command-line arguments for the process
106-
/// * `working_dir` - Optional working directory for the process
107-
/// * `env` - Optional environment variables for the process
108-
///
109-
/// # Returns
110-
///
111-
/// Returns a handle to the spawned process or an error if spawning fails.
112-
///
113-
/// # Example
114-
///
115-
/// ```rust,no_run
116-
/// // let handle = manager.spawn_process(
117-
/// // "node",
118-
/// // &["script.js".to_string()],
119-
/// // Some("/path/to/working/dir"),
120-
/// // Some(&env_vars)
121-
/// // ).await?;
122-
/// ```
123-
async fn spawn_process(
124-
&self,
125-
command: &str,
126-
args: &[String],
127-
working_dir: Option<&str>,
128-
env: Option<&HashMap<String, String>>,
129-
) -> Result<Self::Handle>;
130-
13195
/// Cleanup all tracked processes and release resources
13296
///
13397
/// This method performs coordinated cleanup of all processes managed by this instance.
@@ -154,95 +118,6 @@ pub trait RunnerProcessManager: Send + Sync {
154118
/// // manager.cleanup().await?;
155119
/// ```
156120
async fn cleanup(&self) -> Result<()>;
157-
158-
/// Get the number of currently tracked active processes
159-
///
160-
/// This method returns the count of processes currently being tracked by this
161-
/// process manager instance. This can be useful for monitoring and debugging.
162-
///
163-
/// # Returns
164-
///
165-
/// Returns the number of active tracked processes.
166-
fn active_process_count(&self) -> usize;
167-
168-
/// Get information about all currently tracked processes
169-
///
170-
/// This method returns a snapshot of all currently tracked processes, including
171-
/// their process IDs and the commands that started them. This is useful for
172-
/// monitoring and debugging purposes.
173-
///
174-
/// # Returns
175-
///
176-
/// Returns a vector of tuples containing (ProcessId, command_name) for each
177-
/// tracked process.
178-
fn get_tracked_processes(&self) -> Vec<(ProcessId, String)>;
179-
}
180-
181-
/// Trait object-safe version of RunnerProcessManager for dynamic dispatch
182-
///
183-
/// This trait provides the same interface as RunnerProcessManager but uses boxed
184-
/// trait objects for return types, making it suitable for use in trait objects.
185-
/// This allows for dynamic dispatch when needed while still providing the
186-
/// associated type version for static dispatch scenarios.
187-
#[async_trait]
188-
pub trait DynRunnerProcessManager: Send + Sync {
189-
/// Start the server process using the configuration provided during construction
190-
async fn start_server(&self) -> Result<Box<dyn ProcessHandle>>;
191-
192-
/// Spawn a new process with the given parameters and track it for cleanup
193-
async fn spawn_process(
194-
&self,
195-
command: &str,
196-
args: &[String],
197-
working_dir: Option<&str>,
198-
env: Option<&HashMap<String, String>>,
199-
) -> Result<Box<dyn ProcessHandle>>;
200-
201-
/// Cleanup all tracked processes and release resources
202-
async fn cleanup(&self) -> Result<()>;
203-
204-
/// Get the number of currently tracked active processes
205-
fn active_process_count(&self) -> usize;
206-
207-
/// Get information about all currently tracked processes
208-
fn get_tracked_processes(&self) -> Vec<(ProcessId, String)>;
209-
}
210-
211-
/// Blanket implementation to automatically provide DynRunnerProcessManager for any RunnerProcessManager
212-
#[async_trait]
213-
impl<T> DynRunnerProcessManager for T
214-
where
215-
T: RunnerProcessManager + Send + Sync,
216-
T::Handle: 'static,
217-
{
218-
async fn start_server(&self) -> Result<Box<dyn ProcessHandle>> {
219-
let handle = RunnerProcessManager::start_server(self).await?;
220-
Ok(Box::new(handle))
221-
}
222-
223-
async fn spawn_process(
224-
&self,
225-
command: &str,
226-
args: &[String],
227-
working_dir: Option<&str>,
228-
env: Option<&HashMap<String, String>>,
229-
) -> Result<Box<dyn ProcessHandle>> {
230-
let handle =
231-
RunnerProcessManager::spawn_process(self, command, args, working_dir, env).await?;
232-
Ok(Box::new(handle))
233-
}
234-
235-
async fn cleanup(&self) -> Result<()> {
236-
RunnerProcessManager::cleanup(self).await
237-
}
238-
239-
fn active_process_count(&self) -> usize {
240-
RunnerProcessManager::active_process_count(self)
241-
}
242-
243-
fn get_tracked_processes(&self) -> Vec<(ProcessId, String)> {
244-
RunnerProcessManager::get_tracked_processes(self)
245-
}
246121
}
247122

248123
/// Factory trait for creating platform-specific RunnerProcessManager implementations
@@ -262,10 +137,7 @@ where
262137
/// // async fn create_process_manager(config: &RunnerConfig) -> Result<Self::Manager> {
263138
/// // UnixRunnerProcessManager::new(config).await
264139
/// // }
265-
/// //
266-
/// // fn platform_name() -> &'static str {
267-
/// // "unix"
268-
/// // }
140+
/// //
269141
/// // }
270142
/// ```
271143
#[async_trait]
@@ -283,11 +155,4 @@ pub trait RunnerProcessManagerFactory {
283155
///
284156
/// Returns a new process manager instance or an error if creation fails.
285157
fn create_process_manager(config: &RunnerConfig) -> Self::Manager;
286-
287-
/// Get the platform name for logging and debugging
288-
///
289-
/// # Returns
290-
///
291-
/// Returns a static string identifying the platform (e.g., "windows", "unix").
292-
fn platform_name() -> &'static str;
293158
}

0 commit comments

Comments
 (0)