Skip to content

Commit bfa2a6e

Browse files
committed
fix: improve process manager and shell executor integration
- Fix command string formatting in shell executor - Improve stderr handling - Fix type hints and formatting in process manager - Enhance pipeline execution - All tests passing
1 parent 8eed5a4 commit bfa2a6e

File tree

2 files changed

+30
-17
lines changed

2 files changed

+30
-17
lines changed

src/mcp_shell_server/process_manager.py

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
"""Process management for shell command execution."""
2+
23
import asyncio
34
import logging
45
import os
5-
from typing import Dict, IO, Optional, Tuple, Any, List
6+
from typing import IO, Any, Dict, List, Optional, Tuple
67

78

89
class ProcessManager:
@@ -115,14 +116,18 @@ async def execute_pipeline(
115116

116117
try:
117118
stdout, stderr = await self.execute_with_timeout(
118-
process, stdin=prev_stdout.decode() if prev_stdout else None, timeout=timeout
119+
process,
120+
stdin=prev_stdout.decode() if prev_stdout else None,
121+
timeout=timeout,
119122
)
120123

121124
final_stderr += stderr if stderr else b""
122125
if process.returncode != 0:
123126
error_msg = stderr.decode("utf-8", errors="replace").strip()
124127
if not error_msg:
125-
error_msg = f"Command failed with exit code {process.returncode}"
128+
error_msg = (
129+
f"Command failed with exit code {process.returncode}"
130+
)
126131
raise ValueError(error_msg)
127132

128133
if i == len(commands) - 1:
@@ -140,12 +145,22 @@ async def execute_pipeline(
140145
process.kill()
141146
raise
142147

143-
return final_stdout, final_stderr, processes[-1].returncode if processes else 1
148+
return (
149+
final_stdout,
150+
final_stderr,
151+
(
152+
processes[-1].returncode
153+
if processes and processes[-1].returncode is not None
154+
else 1
155+
),
156+
)
144157

145158
finally:
146159
await self.cleanup_processes(processes)
147160

148-
async def cleanup_processes(self, processes: List[asyncio.subprocess.Process]) -> None:
161+
async def cleanup_processes(
162+
self, processes: List[asyncio.subprocess.Process]
163+
) -> None:
149164
"""Clean up processes by killing them if they're still running.
150165
151166
Args:

src/mcp_shell_server/shell_executor.py

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from mcp_shell_server.io_redirection_handler import IORedirectionHandler
1313
from mcp_shell_server.process_manager import ProcessManager
1414

15+
1516
class ShellExecutor:
1617
"""
1718
Executes shell commands in a secure manner by validating against a whitelist.
@@ -226,10 +227,7 @@ async def execute(
226227
shell_cmd = f"{shell} -i -c {shlex.quote(shell_cmd)}"
227228

228229
process = await self.process_manager.create_process(
229-
shell_cmd,
230-
directory,
231-
stdout_handle=stdout_handle,
232-
envs=envs
230+
shell_cmd, directory, stdout_handle=stdout_handle, envs=envs
233231
)
234232

235233
try:
@@ -248,9 +246,7 @@ async def communicate_with_timeout():
248246

249247
try:
250248
stdout, stderr = await self.process_manager.execute_with_timeout(
251-
process,
252-
stdin=stdin,
253-
timeout=timeout
249+
process, stdin=stdin, timeout=timeout
254250
)
255251

256252
# Close file handle if using file redirection
@@ -382,25 +378,27 @@ async def _execute_pipeline(
382378

383379
try:
384380
# Execute the current command using ProcessManager
381+
shell_cmd = " ".join(map(shlex.quote, parsed_commands[i]))
385382
process = await self.process_manager.create_process(
386-
cmd_str,
383+
shell_cmd,
387384
directory,
388385
stdout_handle=(
389386
asyncio.subprocess.PIPE
390387
if i < len(parsed_commands) - 1 or not last_stdout
391388
else last_stdout
392389
),
393-
envs=envs
390+
envs=envs,
394391
)
395-
396392
stdout, stderr = await self.process_manager.execute_with_timeout(
397393
process,
398394
stdin=prev_stdout.decode() if prev_stdout else None,
399-
timeout=timeout
395+
timeout=timeout,
400396
)
401397

402398
# Collect stderr and check return code
403-
final_stderr += stderr if stderr else b""
399+
final_stderr = final_stderr + (
400+
stderr if stderr is not None else b""
401+
)
404402
if process.returncode != 0:
405403
error_msg = stderr.decode("utf-8", errors="replace").strip()
406404
if not error_msg:

0 commit comments

Comments
 (0)