revert All-Hands-AI#3233 but more logging

Author: tobitege

opendevin/runtime/client/runtime.py

@@ -1,5 +1,4 @@
 import asyncio
-import copy
 import os
 import tempfile
 import uuid
@@ -50,7 +49,6 @@ def __init__(
         plugins: list[PluginRequirement] | None = None,
         container_image: str | None = None,
     ):
-        self.config = copy.deepcopy(config)
         super().__init__(
             config, event_stream, sid, plugins
         )  # will initialize the event stream
@@ -72,6 +70,9 @@ def __init__(
 
         self.container = None
         self.action_semaphore = asyncio.Semaphore(1)  # Ensure one action at a time
+        logger.info(
+            f'EventStreamRuntime `{sid}` __init__ run_as_devin: {self.config.run_as_devin}'
+        )
 
     async def ainit(self, env_vars: dict[str, str] | None = None):
         self.container_image = build_runtime_image(
@@ -145,7 +146,7 @@ async def _init_container(
                 )
                 volumes = None
 
-            logger.info(f'run_as_devin: `{self.config.run_as_devin}`')
+            logger.info(f'_init_container run_as_devin: {self.config.run_as_devin}')
 
             container = self.docker_client.containers.run(
                 self.container_image,

opendevin/runtime/runtime.py

@@ -70,6 +70,9 @@ def __init__(
         self.config = copy.deepcopy(config)
         self.DEFAULT_ENV_VARS = _default_env_vars(config.sandbox)
         atexit.register(self.close_sync)
+        logger.info(
+            f'Runtime `{sid}` __init__ run_as_devin = {self.config.run_as_devin}'
+        )
 
     async def ainit(self, env_vars: dict[str, str] | None = None) -> None:
         """

opendevin/runtime/server/runtime.py

@@ -1,4 +1,3 @@
-import copy
 from typing import Any, Optional
 
 from opendevin.core.config import AppConfig
@@ -44,7 +43,6 @@ def __init__(
         plugins: list[PluginRequirement] | None = None,
         sandbox: Sandbox | None = None,
     ):
-        self.config = copy.deepcopy(config)
         super().__init__(config, event_stream, sid, plugins)
         self.file_store = LocalFileStore(config.workspace_base)
         if sandbox is None:
@@ -54,6 +52,9 @@ def __init__(
             self.sandbox = sandbox
             self._is_external_sandbox = True
         self.browser: BrowserEnv | None = None
+        logger.info(
+            f'ServerRuntime {sid} __init__ run_as_devin = {self.config.run_as_devin}'
+        )
 
     def create_sandbox(self, sid: str = 'default', box_type: str = 'ssh') -> Sandbox:
         if box_type == 'local':

tests/unit/test_runtime.py

@@ -58,7 +58,7 @@ def get_box_classes():
     elif runtime.lower() == 'server':
         return [ServerRuntime]
     else:
-        return [EventStreamRuntime, ServerRuntime]
+        return [ServerRuntime, EventStreamRuntime]
 
 
 # This assures that all tests run together per runtime, not alternating between them,
@@ -141,9 +141,6 @@ async def _load_runtime(
         runtime = ServerRuntime(
             config=config, event_stream=event_stream, sid=sid, plugins=plugins
         )
-        assert (
-            runtime.sandbox.run_as_devin == run_as_devin
-        ), f'run_as_devin in sandbox should be {run_as_devin}'
         await runtime.ainit()
         from opendevin.runtime.tools import (
             RuntimeTool,  # deprecate this after ServerRuntime is deprecated
@@ -792,6 +789,9 @@ async def test_ipython_simple(temp_dir, box_class):
     logger.info(obs, extra={'msg_type': 'OBSERVATION'})
     assert obs.content.strip() == '1'
 
+    await runtime.close()
+    await asyncio.sleep(1)
+
 
 async def _test_ipython_agentskills_fileop_pwd_impl(
     runtime: ServerRuntime | EventStreamRuntime, enable_auto_lint: bool
@@ -904,40 +904,45 @@ async def _test_ipython_agentskills_fileop_pwd_impl(
 
 
 @pytest.mark.asyncio
-async def test_ipython_agentskills_fileop_pwd(temp_dir, box_class, enable_auto_lint):
+async def test_ipython_agentskills_fileop_pwd(
+    temp_dir, box_class, run_as_devin, enable_auto_lint
+):
     """Make sure that cd in bash also update the current working directory in ipython."""
 
     runtime = await _load_runtime(
-        temp_dir, box_class, enable_auto_lint=enable_auto_lint
+        temp_dir, box_class, run_as_devin, enable_auto_lint=enable_auto_lint
     )
     await _test_ipython_agentskills_fileop_pwd_impl(runtime, enable_auto_lint)
+
     await runtime.close()
     await asyncio.sleep(1)
 
 
-@pytest.mark.skipif(
-    TEST_RUNTIME.lower() == 'eventstream',
-    reason='Skip this if we want to test EventStreamRuntime',
-)
 @pytest.mark.skipif(
     os.environ.get('TEST_IN_CI', 'false').lower() == 'true',
     # FIXME: There's some weird issue with the CI environment.
     reason='Skip this if in CI.',
 )
 @pytest.mark.asyncio
 async def test_ipython_agentskills_fileop_pwd_agnostic_sandbox(
-    temp_dir, enable_auto_lint, container_image
+    temp_dir, box_class, run_as_devin, enable_auto_lint, container_image
 ):
-    """Make sure that cd in bash also update the current working directory in ipython."""
+    """Make sure that cd in bash also updates the current working directory in iPython."""
+
+    # NOTE: we only test for ServerRuntime, since EventStreamRuntime
+    # is image agnostic by design.
+    if box_class != 'server':
+        pytest.skip('Skip this if box_class is not server')
 
     runtime = await _load_runtime(
         temp_dir,
-        # NOTE: we only test for ServerRuntime, since EventStreamRuntime is image agnostic by design.
-        ServerRuntime,
+        box_class,
+        run_as_devin,
         enable_auto_lint=enable_auto_lint,
         container_image=container_image,
     )
     await _test_ipython_agentskills_fileop_pwd_impl(runtime, enable_auto_lint)
+
     await runtime.close()
     await asyncio.sleep(1)
 
@@ -1030,6 +1035,9 @@ async def test_copy_single_file(temp_dir, box_class):
     assert obs.exit_code == 0
     assert 'Hello, World!' in obs.content
 
+    await runtime.close()
+    await asyncio.sleep(1)
+
 
 def _create_test_dir_with_files(host_temp_dir):
     os.mkdir(os.path.join(host_temp_dir, 'test_dir'))
@@ -1077,6 +1085,9 @@ async def test_copy_directory_recursively(temp_dir, box_class):
     assert obs.exit_code == 0
     assert 'File 1 content' in obs.content
 
+    await runtime.close()
+    await asyncio.sleep(1)
+
 
 @pytest.mark.asyncio
 async def test_copy_to_non_existent_directory(temp_dir, box_class):
@@ -1096,6 +1107,9 @@ async def test_copy_to_non_existent_directory(temp_dir, box_class):
     assert obs.exit_code == 0
     assert 'Hello, World!' in obs.content
 
+    await runtime.close()
+    await asyncio.sleep(1)
+
 
 @pytest.mark.asyncio
 async def test_overwrite_existing_file(temp_dir, box_class):
@@ -1131,6 +1145,9 @@ async def test_overwrite_existing_file(temp_dir, box_class):
     assert obs.exit_code == 0
     assert 'Hello, World!' in obs.content
 
+    await runtime.close()
+    await asyncio.sleep(1)
+
 
 @pytest.mark.asyncio
 async def test_copy_non_existent_file(temp_dir, box_class):
@@ -1148,3 +1165,6 @@ async def test_copy_non_existent_file(temp_dir, box_class):
     logger.info(obs, extra={'msg_type': 'OBSERVATION'})
     assert isinstance(obs, CmdOutputObservation)
     assert obs.exit_code != 0  # File should not exist
+
+    await runtime.close()
+    await asyncio.sleep(1)