Skip to content

Commit 22b279f

Browse files
committed
Merge branch 'develop'
2 parents b070bc3 + 947935b commit 22b279f

File tree

3 files changed

+87
-22
lines changed

3 files changed

+87
-22
lines changed

src/mcp_shell_server/server.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,14 @@ async def run_tool(self, arguments: dict) -> Sequence[TextContent]:
9494
if result.get("error"):
9595
raise ValueError(result["error"])
9696

97+
# Add stdout if present
9798
if result.get("stdout"):
9899
content.append(TextContent(type="text", text=result["stdout"]))
99-
if result.get("stderr"):
100-
content.append(TextContent(type="text", text=result["stderr"]))
100+
101+
# Add stderr if present (filter out specific messages)
102+
stderr = result.get("stderr")
103+
if stderr and "cannot set terminal process group" not in stderr:
104+
content.append(TextContent(type="text", text=stderr))
101105

102106
except asyncio.TimeoutError as e:
103107
raise ValueError(f"Command timed out after {timeout} seconds") from e

tests/test_server.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import asyncio
12
import os
23
import tempfile
34

@@ -7,6 +8,67 @@
78
from mcp_shell_server.server import call_tool, list_tools
89

910

11+
# Mock process class
12+
class MockProcess:
13+
def __init__(self, stdout=None, stderr=None, returncode=0):
14+
self.stdout = stdout
15+
self.stderr = stderr
16+
self.returncode = returncode
17+
self._input = None
18+
19+
async def communicate(self, input=None):
20+
self._input = input
21+
if self._input and not isinstance(self._input, bytes):
22+
self._input = self._input.encode("utf-8")
23+
24+
# For cat command, echo back the input
25+
if self.stdout is None and self._input:
26+
return self._input, self.stderr
27+
28+
if isinstance(self.stdout, int):
29+
self.stdout = str(self.stdout).encode("utf-8")
30+
if self.stdout is None:
31+
self.stdout = b""
32+
if self.stderr is None:
33+
self.stderr = b""
34+
return self.stdout, self.stderr
35+
36+
async def wait(self):
37+
return self.returncode
38+
39+
def kill(self):
40+
pass
41+
42+
43+
def setup_mock_subprocess(monkeypatch):
44+
"""Set up mock subprocess to avoid interactive shell warnings"""
45+
46+
async def mock_create_subprocess_shell(
47+
cmd, stdin=None, stdout=None, stderr=None, env=None, cwd=None
48+
):
49+
# Return appropriate output based on command
50+
if "echo" in cmd:
51+
return MockProcess(stdout=b"hello world\n", stderr=b"", returncode=0)
52+
elif "pwd" in cmd:
53+
return MockProcess(stdout=cwd.encode() + b"\n", stderr=b"", returncode=0)
54+
elif "cat" in cmd:
55+
return MockProcess(
56+
stdout=None, stderr=b"", returncode=0
57+
) # Will echo back stdin
58+
elif "ps" in cmd:
59+
return MockProcess(stdout=b"bash\n", stderr=b"", returncode=0)
60+
elif "env" in cmd:
61+
return MockProcess(stdout=b"TEST_ENV=value\n", stderr=b"", returncode=0)
62+
elif "sleep" in cmd:
63+
return MockProcess(stdout=b"", stderr=b"", returncode=0)
64+
else:
65+
return MockProcess(stdout=b"", stderr=b"", returncode=0)
66+
67+
monkeypatch.setattr(
68+
asyncio, "create_subprocess_shell", mock_create_subprocess_shell
69+
)
70+
71+
1072
@pytest.fixture
1173
def temp_test_dir():
1274
"""Create a temporary directory for testing"""
@@ -62,6 +124,7 @@ async def test_call_tool_valid_command(monkeypatch, temp_test_dir):
62124
@pytest.mark.asyncio
63125
async def test_call_tool_with_stdin(monkeypatch, temp_test_dir):
64126
"""Test command execution with stdin"""
127+
setup_mock_subprocess(monkeypatch)
65128
monkeypatch.setenv("ALLOW_COMMANDS", "cat")
66129
result = await call_tool(
67130
"shell_execute",
@@ -237,6 +300,22 @@ async def test_disallowed_command(monkeypatch):
237300
@pytest.mark.asyncio
238301
async def test_call_tool_with_stderr(monkeypatch):
239302
"""Test command execution with stderr output"""
303+
304+
async def mock_create_subprocess_shell(
305+
cmd, stdin=None, stdout=None, stderr=None, env=None, cwd=None
306+
):
307+
# Return mock process with stderr for ls command
308+
if "ls" in cmd:
309+
return MockProcess(
310+
stdout=b"",
311+
stderr=b"ls: cannot access '/nonexistent/directory': No such file or directory\n",
312+
returncode=2,
313+
)
314+
return MockProcess(stdout=b"", stderr=b"", returncode=0)
315+
316+
monkeypatch.setattr(
317+
asyncio, "create_subprocess_shell", mock_create_subprocess_shell
318+
)
240319
monkeypatch.setenv("ALLOW_COMMANDS", "ls")
241320
result = await call_tool(
242321
"shell_execute",
@@ -327,6 +406,7 @@ def stdio_server_impl():
327406
@pytest.mark.asyncio
328407
async def test_shell_startup(monkeypatch, temp_test_dir):
329408
"""Test shell startup and environment"""
409+
setup_mock_subprocess(monkeypatch)
330410
monkeypatch.setenv("ALLOW_COMMANDS", "ps")
331411
result = await call_tool(
332412
"shell_execute",
@@ -339,6 +419,7 @@ async def test_shell_startup(monkeypatch, temp_test_dir):
339419
@pytest.mark.asyncio
340420
async def test_environment_variables(monkeypatch, temp_test_dir):
341421
"""Test to check environment variables during test execution"""
422+
setup_mock_subprocess(monkeypatch)
342423
monkeypatch.setenv("ALLOW_COMMANDS", "env")
343424
result = await call_tool(
344425
"shell_execute",

tests/test_shell_executor.py

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -417,26 +417,6 @@ async def test_complex_pipeline_with_redirections(executor, temp_test_dir, monke
417417
assert lines[1].strip() == "WORLD"
418418

419419

420-
@pytest.mark.asyncio
421-
async def test_directory_permissions(executor, temp_test_dir, monkeypatch):
422-
"""Test command execution with directory permission issues"""
423-
clear_env(monkeypatch)
424-
monkeypatch.setenv("ALLOW_COMMANDS", "ls")
425-
426-
# Create a directory with no execute permission
427-
no_exec_dir = os.path.join(temp_test_dir, "no_exec_dir")
428-
os.mkdir(no_exec_dir)
429-
os.chmod(no_exec_dir, 0o600) # Remove execute permission
430-
431-
try:
432-
result = await executor.execute(["ls"], directory=no_exec_dir)
433-
assert result["error"] == f"Directory is not accessible: {no_exec_dir}"
434-
assert result["status"] == 1
435-
finally:
436-
# Restore permissions for cleanup
437-
os.chmod(no_exec_dir, 0o700)
438-
439-
440420
def test_validate_redirection_syntax(executor):
441421
"""Test validation of redirection syntax with various input combinations"""
442422
# Valid cases

0 commit comments

Comments
 (0)