feat: update command execution to use isolated working directory and add tests

Author: laysakura

CHANGELOG.md

@@ -11,8 +11,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Changed command execution working directory in wish-command-execution:
+  - Modified BashBackend to execute commands in `/app/{runId}/` directory
+  - Added automatic directory creation with `os.makedirs`
+  - Updated tests to verify correct working directory is used
+  - Improved isolation between command execution and application source code
+
 ### Fixed
 
+- Fixed OpenAI API errors in wish-log-analysis-api:
+  - Isolated command execution from application source code to prevent large outputs
+  - Prevented `.venv` and other source code files from being included in command output
+
 ### Removed
 
 

wish-command-execution/src/wish_command_execution/backend/bash.py

@@ -128,6 +128,10 @@ async def execute_command(self, wish: Wish, command: str, cmd_num: int, log_file
             cmd_num: The command number.
             log_files: The log files to write output to.
             timeout_sec: The timeout in seconds for this command.
+            
+        Note:
+            Commands are executed in the working directory /app/{run_id}/ to isolate
+            command execution from the application source code.
         """
         # Create command result
         result = CommandResult.create(cmd_num, command, log_files, timeout_sec)
@@ -139,13 +143,21 @@ async def execute_command(self, wish: Wish, command: str, cmd_num: int, log_file
 
         with open(log_files.stdout, "w") as stdout_file, open(log_files.stderr, "w") as stderr_file:
             try:
+                # 作業ディレクトリを設定
+                cwd = f"/app/{self.run_id or wish.id}/"
+                
+                # ディレクトリが存在することを確認
+                import os
+                os.makedirs(cwd, exist_ok=True)
+                
                 # Start the process (this is still synchronous, but the interface is async)
                 process = subprocess.Popen(
                     command,
                     stdout=stdout_file,
                     stderr=stderr_file,
                     shell=True,
-                    text=True
+                    text=True,
+                    cwd=cwd  # 作業ディレクトリを指定
                 )
 
                 # Store in running commands dict with timeout information

wish-command-execution/tests/backend/test_bash_backend.py

@@ -36,7 +36,8 @@ def log_files(self):
     @pytest.mark.asyncio
     @patch("subprocess.Popen")
     @patch("builtins.open")
-    async def test_execute_command(self, mock_open, mock_popen, backend, wish, log_files):
+    @patch("os.makedirs")
+    async def test_execute_command(self, mock_makedirs, mock_open, mock_popen, backend, wish, log_files):
         """Test execute_command method.
 
         This test verifies that the execute_command method correctly executes
@@ -57,13 +58,20 @@ async def test_execute_command(self, mock_open, mock_popen, backend, wish, log_f
         timeout_sec = 60
         await backend.execute_command(wish, cmd, cmd_num, log_files, timeout_sec)
 
-        # Verify that Popen was called with the expected command
+        # Expected working directory
+        expected_cwd = f"/app/{backend.run_id or wish.id}/"
+        
+        # Verify that os.makedirs was called to ensure the directory exists
+        mock_makedirs.assert_called_once_with(expected_cwd, exist_ok=True)
+        
+        # Verify that Popen was called with the expected command and cwd
         mock_popen.assert_any_call(
             cmd,
             stdout=mock_stdout,
             stderr=mock_stderr,
             shell=True,
-            text=True
+            text=True,
+            cwd=expected_cwd
         )
 
         # Verify that the command result was added to the wish
@@ -78,7 +86,8 @@ async def test_execute_command(self, mock_open, mock_popen, backend, wish, log_f
     @pytest.mark.asyncio
     @patch("subprocess.Popen")
     @patch("builtins.open")
-    async def test_execute_command_subprocess_error(self, mock_open, mock_popen, backend, wish, log_files):
+    @patch("os.makedirs")
+    async def test_execute_command_subprocess_error(self, mock_makedirs, mock_open, mock_popen, backend, wish, log_files):
         """Test execute_command method with subprocess error."""
         # Set up the mock Popen to raise a subprocess error
         mock_popen.side_effect = subprocess.SubprocessError("Mock error")
@@ -93,6 +102,12 @@ async def test_execute_command_subprocess_error(self, mock_open, mock_popen, bac
         cmd_num = 1
         timeout_sec = 60
         await backend.execute_command(wish, cmd, cmd_num, log_files, timeout_sec)
+        
+        # Expected working directory
+        expected_cwd = f"/app/{backend.run_id or wish.id}/"
+        
+        # Verify that os.makedirs was called to ensure the directory exists
+        mock_makedirs.assert_called_once_with(expected_cwd, exist_ok=True)
 
         # Verify that the command result was added to the wish
         assert len(wish.command_results) == 1
@@ -177,7 +192,8 @@ async def test_cancel_command_not_running(self, backend, wish):
     @pytest.mark.asyncio
     @patch("subprocess.Popen")
     @patch("builtins.open")
-    async def test_execute_command_without_variable_replacement(self, mock_open, mock_popen, backend, wish, log_files):
+    @patch("os.makedirs")
+    async def test_execute_command_without_variable_replacement(self, mock_makedirs, mock_open, mock_popen, backend, wish, log_files):
         """Test execute_command method without variable replacement."""
         # Set up the mock Popen
         mock_process = MagicMock()
@@ -194,13 +210,21 @@ async def test_execute_command_without_variable_replacement(self, mock_open, moc
         timeout_sec = 60
         await backend.execute_command(wish, cmd, cmd_num, log_files, timeout_sec)
 
-        # Verify that Popen was called with the original command
+        # Expected working directory
+        expected_cwd = f"/app/{backend.run_id or wish.id}/"
+        
+        # Verify that os.makedirs was called to ensure the directory exists
+        mock_makedirs.assert_called_once_with(expected_cwd, exist_ok=True)
+        
+        # Verify that Popen was called with the original command and correct cwd
         mock_popen.assert_any_call(
             cmd,
             stdout=mock_stdout,
             stderr=mock_stderr,
             shell=True,
-            text=True
+            text=True,
+            cwd=expected_cwd
         )
         args, kwargs = mock_popen.call_args
         assert args[0] == cmd
+        assert kwargs['cwd'] == expected_cwd