diff --git a/opendevin/core/logger.py b/opendevin/core/logger.py index 5bde01847504..e5a74206f016 100644 --- a/opendevin/core/logger.py +++ b/opendevin/core/logger.py @@ -9,7 +9,7 @@ from termcolor import colored DISABLE_COLOR_PRINTING = False -DEBUG = False +DEBUG = os.getenv('DEBUG', 'False').lower() in ['true', '1', 'yes'] ColorType = Literal[ 'red', diff --git a/opendevin/runtime/client/client.py b/opendevin/runtime/client/client.py index 8d7c9f3ccc6d..41187d2b9894 100644 --- a/opendevin/runtime/client/client.py +++ b/opendevin/runtime/client/client.py @@ -46,6 +46,7 @@ Plugin, ) from opendevin.runtime.server.files import insert_lines, read_lines +from opendevin.runtime.utils import split_bash_commands app = FastAPI() @@ -79,7 +80,7 @@ def _init_bash_shell(self, work_dir: str) -> None: r'\[PEXPECT_BEGIN\] ([a-z0-9_-]*)@([a-zA-Z0-9.-]*):(.+) \[PEXPECT_END\]' ) - self.shell.sendline(f'export PS1="{self.__bash_PS1}"') + self.shell.sendline(f'export PS1="{self.__bash_PS1}"; export PS2=""') self.shell.expect(self.__bash_expect_regex) self.shell.sendline(f'cd {work_dir}') @@ -87,6 +88,15 @@ def _init_bash_shell(self, work_dir: str) -> None: def _get_bash_prompt(self): ps1 = self.shell.after + + # begin at the last occurence of '[PEXPECT_BEGIN]'. + # In multi-line bash commands, the prompt will be repeated + # and the matched regex captures all of them + # - we only want the last one (newest prompt) + _begin_pos = ps1.rfind('[PEXPECT_BEGIN]') + if _begin_pos != -1: + ps1 = ps1[_begin_pos:] + # parse the ps1 to get username, hostname, and working directory matched = re.match(self.__bash_expect_regex, ps1) assert ( @@ -102,7 +112,7 @@ def _get_bash_prompt(self): prompt += '$' return prompt + ' ' - def _execute_bash(self, command, keep_prompt: bool = True) -> tuple[str, int]: + def _execute_bash(self, command: str, keep_prompt: bool = True) -> tuple[str, int]: logger.debug(f'Executing command: {command}') self.shell.sendline(command) self.shell.expect(self.__bash_expect_regex) @@ -129,10 +139,22 @@ async def run_action(self, action) -> Observation: async def run(self, action: CmdRunAction) -> CmdOutputObservation: try: - output, exit_code = self._execute_bash(action.command) + commands = split_bash_commands(action.command) + all_output = '' + for command in commands: + output, exit_code = self._execute_bash(command) + if all_output: + # previous output already exists with prompt "user@hostname:working_dir #"" + # we need to add the command to the previous output, + # so model knows the following is the output of another action) + all_output = all_output.rstrip() + ' ' + command + '\r\n' + + all_output += str(output) + '\r\n' + if exit_code != 0: + break return CmdOutputObservation( command_id=-1, - content=str(output), + content=all_output.rstrip('\r\n'), command=action.command, exit_code=exit_code, ) diff --git a/opendevin/runtime/client/runtime.py b/opendevin/runtime/client/runtime.py index 100d18df9036..5c39c3a59189 100644 --- a/opendevin/runtime/client/runtime.py +++ b/opendevin/runtime/client/runtime.py @@ -58,7 +58,7 @@ def __init__( # TODO: We can switch to aiodocker when `get_od_sandbox_image` is updated to use aiodocker self.docker_client: docker.DockerClient = self._init_docker_client() self.container_image = ( - config.sandbox.container_image + self.config.sandbox.container_image if container_image is None else container_image ) @@ -103,7 +103,7 @@ def _init_docker_client() -> docker.DockerClient: async def _init_container( self, sandbox_workspace_dir: str, - mount_dir: str, + mount_dir: str | None = None, plugins: list[PluginRequirement] | None = None, ): try: @@ -124,6 +124,14 @@ async def _init_container( else: port_mapping = {f'{self._port}/tcp': self._port} + if mount_dir is not None: + volumes = {mount_dir: {'bind': sandbox_workspace_dir, 'mode': 'rw'}} + else: + logger.warn( + 'Mount dir is not set, will not mount the workspace directory to the container.' + ) + volumes = None + container = self.docker_client.containers.run( self.container_image, command=( @@ -139,7 +147,7 @@ async def _init_container( name=self.container_name, detach=True, environment={'DEBUG': 'true'} if self.config.debug else None, - volumes={mount_dir: {'bind': sandbox_workspace_dir, 'mode': 'rw'}}, + volumes=volumes, ) logger.info(f'Container started. Server url: {self.api_url}') return container diff --git a/opendevin/runtime/runtime.py b/opendevin/runtime/runtime.py index 0be6d97d0af8..b38de62f3795 100644 --- a/opendevin/runtime/runtime.py +++ b/opendevin/runtime/runtime.py @@ -33,13 +33,13 @@ from opendevin.storage import FileStore -def _default_env_vars(config: SandboxConfig) -> dict[str, str]: +def _default_env_vars(sandbox_config: SandboxConfig) -> dict[str, str]: ret = {} for key in os.environ: if key.startswith('SANDBOX_ENV_'): sandbox_key = key.removeprefix('SANDBOX_ENV_') ret[sandbox_key] = os.environ[key] - if config.enable_auto_lint: + if sandbox_config.enable_auto_lint: ret['ENABLE_AUTO_LINT'] = 'true' return ret diff --git a/opendevin/runtime/server/runtime.py b/opendevin/runtime/server/runtime.py index 04b6ee57fe05..392c0bafd5da 100644 --- a/opendevin/runtime/server/runtime.py +++ b/opendevin/runtime/server/runtime.py @@ -115,7 +115,7 @@ async def run(self, action: CmdRunAction) -> Observation: async def run_ipython(self, action: IPythonRunCellAction) -> Observation: self._run_command( - ("cat > /tmp/opendevin_jupyter_temp.py <<'EOL'\n" f'{action.code}\n' 'EOL'), + f"cat > /tmp/opendevin_jupyter_temp.py <<'EOL'\n{action.code}\nEOL" ) # run the code diff --git a/opendevin/runtime/utils/bash.py b/opendevin/runtime/utils/bash.py index 9dd7a60422c5..a65342d5ffa3 100644 --- a/opendevin/runtime/utils/bash.py +++ b/opendevin/runtime/utils/bash.py @@ -1,87 +1,49 @@ -def split_bash_commands(commands): - # States - NORMAL = 0 - IN_SINGLE_QUOTE = 1 - IN_DOUBLE_QUOTE = 2 - IN_HEREDOC = 3 - - state = NORMAL - heredoc_trigger = None - result = [] - current_command: list[str] = [] - - i = 0 - while i < len(commands): - char = commands[i] - - if state == NORMAL: - if char == "'": - state = IN_SINGLE_QUOTE - elif char == '"': - state = IN_DOUBLE_QUOTE - elif char == '\\': - # Check if this is escaping a newline - if i + 1 < len(commands) and commands[i + 1] == '\n': - i += 1 # Skip the newline - # Continue with the next line as part of the same command - i += 1 # Move to the first character of the next line - continue - elif char == '\n': - if not heredoc_trigger and current_command: - result.append(''.join(current_command).strip()) - current_command = [] - elif char == '<' and commands[i : i + 2] == '<<': - # Detect heredoc - state = IN_HEREDOC - i += 2 # Skip '<<' - while commands[i] == ' ': - i += 1 - start = i - while commands[i] not in [' ', '\n']: - i += 1 - heredoc_trigger = commands[start:i] - current_command.append(commands[start - 2 : i]) # Include '<<' - continue # Skip incrementing i at the end of the loop - current_command.append(char) - - elif state == IN_SINGLE_QUOTE: - current_command.append(char) - if char == "'" and commands[i - 1] != '\\': - state = NORMAL +import bashlex - elif state == IN_DOUBLE_QUOTE: - current_command.append(char) - if char == '"' and commands[i - 1] != '\\': - state = NORMAL +from opendevin.core.logger import opendevin_logger as logger - elif state == IN_HEREDOC: - current_command.append(char) - if ( - char == '\n' - and heredoc_trigger - and commands[i + 1 : i + 1 + len(heredoc_trigger) + 1] - == heredoc_trigger + '\n' - ): - # Check if the next line starts with the heredoc trigger followed by a newline - i += ( - len(heredoc_trigger) + 1 - ) # Move past the heredoc trigger and newline - current_command.append( - heredoc_trigger + '\n' - ) # Include the heredoc trigger and newline - result.append(''.join(current_command).strip()) - current_command = [] - heredoc_trigger = None - state = NORMAL - continue - - i += 1 - - # Add the last command if any - if current_command: - result.append(''.join(current_command).strip()) - - # Remove any empty strings from the result - result = [cmd for cmd in result if cmd] +def split_bash_commands(commands): + try: + parsed = bashlex.parse(commands) + except bashlex.errors.ParsingError as e: + logger.error( + f'Failed to parse bash commands\n[input]: {commands}\n[error]: {e}' + ) + # If parsing fails, return the original commands + return [commands] + + result: list[str] = [] + last_end = 0 + + for node in parsed: + start, end = node.pos + + # Include any text between the last command and this one + if start > last_end: + between = commands[last_end:start] + logger.debug(f'BASH PARSING between: {between}') + if result: + result[-1] += between.rstrip() + elif between.strip(): + # THIS SHOULD NOT HAPPEN + result.append(between.rstrip()) + + # Extract the command, preserving original formatting + command = commands[start:end].rstrip() + logger.debug(f'BASH PARSING command: {command}') + result.append(command) + + last_end = end + + # Add any remaining text after the last command to the last command + remaining = commands[last_end:].rstrip() + logger.debug(f'BASH PARSING remaining: {remaining}') + if last_end < len(commands) and result: + result[-1] += remaining + logger.debug(f'BASH PARSING result[-1] += remaining: {result[-1]}') + elif last_end < len(commands): + if remaining: + result.append(remaining) + logger.debug(f'BASH PARSING result.append(remaining): {result[-1]}') return result diff --git a/poetry.lock b/poetry.lock index 19b60549653b..12d95380695c 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. [[package]] name = "aenum" @@ -398,6 +398,17 @@ files = [ {file = "backoff-2.2.1.tar.gz", hash = "sha256:03f829f5bb1923180821643f8753b0502c3b682293992485b0eef2807afa5cba"}, ] +[[package]] +name = "bashlex" +version = "0.18" +description = "Python parser for bash" +optional = false +python-versions = ">=2.7, !=3.0, !=3.1, !=3.2, !=3.3, !=3.4" +files = [ + {file = "bashlex-0.18-py2.py3-none-any.whl", hash = "sha256:91d73a23a3e51711919c1c899083890cdecffc91d8c088942725ac13e9dcfffa"}, + {file = "bashlex-0.18.tar.gz", hash = "sha256:5bb03a01c6d5676338c36fd1028009c8ad07e7d61d8a1ce3f513b7fff52796ee"}, +] + [[package]] name = "bcrypt" version = "4.1.3" @@ -9109,4 +9120,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "^3.11" -content-hash = "e1520f1342ab527bc3bb2619f8909cbdddeb227c14614eb3d82e133961f1f4d2" +content-hash = "6d6cfaf3a614a4bf766d9a0e886e82dc9f8cfb8bf08a642f0207f260e72dd6da" diff --git a/pyproject.toml b/pyproject.toml index 5496eddec6de..061f0f99a6d1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,7 @@ pathspec = "^0.12.1" google-cloud-aiplatform = "*" grep-ast = "0.3.2" tree-sitter = "0.21.3" +bashlex = "^0.18" [tool.poetry.group.llama-index.dependencies] llama-index = "*" @@ -72,6 +73,7 @@ reportlab = "*" [tool.coverage.run] concurrency = ["gevent"] + [tool.poetry.group.runtime.dependencies] jupyterlab = "*" notebook = "*" @@ -105,6 +107,7 @@ ignore = ["D1"] [tool.ruff.lint.pydocstyle] convention = "google" + [tool.poetry.group.evaluation.dependencies] streamlit = "*" whatthepatch = "*" diff --git a/tests/unit/test_bash_parsing.py b/tests/unit/test_bash_parsing.py new file mode 100644 index 000000000000..37c40f3dd889 --- /dev/null +++ b/tests/unit/test_bash_parsing.py @@ -0,0 +1,283 @@ +import pytest + +from opendevin.runtime.utils.bash import split_bash_commands + + +def test_split_commands_util(): + cmds = [ + 'ls -l', + 'echo -e "hello\nworld"', + """ +echo -e "hello it\\'s me" +""".strip(), + """ +echo \\ + -e 'hello' \\ + -v +""".strip(), + """ +echo -e 'hello\\nworld\\nare\\nyou\\nthere?' +""".strip(), + """ +echo -e 'hello +world +are +you\\n +there?' +""".strip(), + """ +echo -e 'hello +world " +' +""".strip(), + """ +kubectl apply -f - < /tmp/opendevin_jupyter_temp.py <<'EOL' + print('Hello, `World`! + ') + EOL + [error]: here-document at line 0 delimited by end-of-file (wanted "'EOL'") (position 75) + + TODO: remove this tests after the deprecation of ServerRuntime + """ + + code = "print('Hello, `World`!\n')" + input_commands = f"""cat > /tmp/opendevin_jupyter_temp.py <<'EOL' +{code} +EOL""" + expected_output = [f"cat > /tmp/opendevin_jupyter_temp.py <<'EOL'\n{code}\nEOL"] + assert split_bash_commands(input_commands) == expected_output + + +def test_backslash_continuation(): + input_commands = """ +echo "This is a long \ +command that spans \ +multiple lines" +echo "Next command" +""" + expected_output = [ + 'echo "This is a long command that spans multiple lines"', + 'echo "Next command"', + ] + assert split_bash_commands(input_commands) == expected_output + + +def test_comments(): + input_commands = """ +echo "Hello" # This is a comment +# This is another comment +ls -l +""" + expected_output = [ + 'echo "Hello" # This is a comment\n# This is another comment', + 'ls -l', + ] + assert split_bash_commands(input_commands) == expected_output + + +def test_complex_quoting(): + input_commands = """ +echo "This is a \\"quoted\\" string" +echo 'This is a '\''single-quoted'\'' string' +echo "Mixed 'quotes' in \\"double quotes\\"" +""" + expected_output = [ + 'echo "This is a \\"quoted\\" string"', + "echo 'This is a '''single-quoted''' string'", + 'echo "Mixed \'quotes\' in \\"double quotes\\""', + ] + assert split_bash_commands(input_commands) == expected_output + + +def test_invalid_syntax(): + invalid_inputs = [ + 'echo "Unclosed quote', + "echo 'Unclosed quote", + 'cat < server.log 2>&1 &') + logger.info(action_cmd, extra={'msg_type': 'ACTION'}) + obs = await runtime.run_action(action_cmd) + logger.info(obs, extra={'msg_type': 'OBSERVATION'}) + + assert isinstance(obs, CmdOutputObservation) + assert obs.exit_code == 0 + assert '[1]' in obs.content + + action_browse = BrowseURLAction(url='http://localhost:8000') + logger.info(action_browse, extra={'msg_type': 'ACTION'}) + obs = await runtime.run_action(action_browse) + logger.info(obs, extra={'msg_type': 'OBSERVATION'}) + + assert isinstance(obs, BrowserOutputObservation) + assert 'http://localhost:8000' in obs.url + assert obs.status_code == 200 + assert not obs.error + assert obs.open_pages_urls == ['http://localhost:8000/'] + assert obs.active_page_index == 0 + assert obs.last_browser_action == 'goto("http://localhost:8000")' + assert obs.last_browser_action_error == '' + assert 'Directory listing for /' in obs.content + assert 'server.log' in obs.content + + await runtime.close() + + +@pytest.mark.asyncio +async def test_multiline_commands(temp_dir, box_class): + cmds = [ + 'ls -l', + 'echo -e "hello\nworld"', + """ +echo -e "hello it\\'s me" +""".strip(), + """ +echo \\ + -e 'hello' \\ + -v +""".strip(), + """ +echo -e 'hello\\nworld\\nare\\nyou\\nthere?' +""".strip(), + """ +echo -e 'hello +world +are +you\\n +there?' +""".strip(), + """ +echo -e 'hello +world " +' +""".strip(), + ] + joined_cmds = '\n'.join(cmds) + + runtime = await _load_runtime(temp_dir, box_class) + + action = CmdRunAction(command=joined_cmds) + logger.info(action, extra={'msg_type': 'ACTION'}) + obs = await runtime.run_action(action) + logger.info(obs, extra={'msg_type': 'OBSERVATION'}) + + assert isinstance(obs, CmdOutputObservation) + assert obs.exit_code == 0, 'The exit code should be 0.' + + assert 'total 0' in obs.content + assert 'hello\r\nworld' in obs.content + assert "hello it\\'s me" in obs.content + assert 'hello -v' in obs.content + assert 'hello\r\nworld\r\nare\r\nyou\r\nthere?' in obs.content + assert 'hello\r\nworld\r\nare\r\nyou\r\n\r\nthere?' in obs.content + assert 'hello\r\nworld "\r\n' in obs.content + + await runtime.close() + await asyncio.sleep(1) + + +@pytest.mark.asyncio +async def test_no_ps2_in_output(temp_dir, box_class): + """Test that the PS2 sign is not added to the output of a multiline command.""" + runtime = await _load_runtime(temp_dir, box_class) + + action = CmdRunAction(command='echo -e "hello\nworld"') + logger.info(action, extra={'msg_type': 'ACTION'}) + obs = await runtime.run_action(action) + logger.info(obs, extra={'msg_type': 'OBSERVATION'}) + + if box_class == ServerRuntime: + # the extra PS2 '>' is NOT handled by the ServerRuntime + assert 'hello\r\nworld' in obs.content + assert '>' in obs.content + assert obs.content.count('>') == 1 + else: + assert 'hello\r\nworld' in obs.content + assert '>' not in obs.content + + +@pytest.mark.asyncio +async def test_multiline_command_loop(temp_dir, box_class): + # https://github.com/OpenDevin/OpenDevin/issues/3143 + + runtime = await _load_runtime(temp_dir, box_class) + + init_cmd = """ +mkdir -p _modules && \ +for month in {01..04}; do + for day in {01..05}; do + touch "_modules/2024-${month}-${day}-sample.md" + done +done +echo "created files" +""" + action = CmdRunAction(command=init_cmd) + logger.info(action, extra={'msg_type': 'ACTION'}) + obs = await runtime.run_action(action) + logger.info(obs, extra={'msg_type': 'OBSERVATION'}) + + assert isinstance(obs, CmdOutputObservation) + assert obs.exit_code == 0, 'The exit code should be 0.' + assert 'created files' in obs.content + + follow_up_cmd = """ +for file in _modules/*.md; do + new_date=$(echo $file | sed -E 's/2024-(01|02|03|04)-/2024-/;s/2024-01/2024-08/;s/2024-02/2024-09/;s/2024-03/2024-10/;s/2024-04/2024-11/') + mv "$file" "$new_date" +done +echo "success" +""" + action = CmdRunAction(command=follow_up_cmd) + logger.info(action, extra={'msg_type': 'ACTION'}) + obs = await runtime.run_action(action) + logger.info(obs, extra={'msg_type': 'OBSERVATION'}) + + assert isinstance(obs, CmdOutputObservation) + assert obs.exit_code == 0, 'The exit code should be 0.' + assert 'success' in obs.content + + await runtime.close() + await asyncio.sleep(1) diff --git a/tests/unit/test_sandbox.py b/tests/unit/test_sandbox.py index eebcdf037d1f..dd78fef4c903 100644 --- a/tests/unit/test_sandbox.py +++ b/tests/unit/test_sandbox.py @@ -7,7 +7,6 @@ from opendevin.core.config import AppConfig, SandboxConfig from opendevin.runtime.docker.ssh_box import DockerSSHBox from opendevin.runtime.plugins import AgentSkillsRequirement, JupyterRequirement -from opendevin.runtime.utils import split_bash_commands def create_docker_box_from_app_config( @@ -41,62 +40,6 @@ def temp_dir(monkeypatch): yield temp_dir -def test_split_commands(): - cmds = [ - 'ls -l', - 'echo -e "hello\nworld"', - """ -echo -e 'hello it\\'s me' -""".strip(), - """ -echo \\ - -e 'hello' \\ - -v -""".strip(), - """ -echo -e 'hello\\nworld\\nare\\nyou\\nthere?' -""".strip(), - """ -echo -e 'hello -world -are -you\\n -there?' -""".strip(), - """ -echo -e 'hello -world " -' -""".strip(), - """ -kubectl apply -f - <