Skip to content

Commit

Permalink
Split bash commands by the new line character (#4462)
Browse files Browse the repository at this point in the history
  • Loading branch information
tofarr authored Oct 24, 2024
1 parent 615b94c commit 90e2bf4
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 27 deletions.
7 changes: 0 additions & 7 deletions frontend/src/services/actions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { addAssistantMessage, addUserMessage } from "#/state/chatSlice";
import { setCode, setActiveFilepath } from "#/state/codeSlice";
import { appendInput } from "#/state/commandSlice";
import { appendJupyterInput } from "#/state/jupyterSlice";
import {
ActionSecurityRisk,
Expand Down Expand Up @@ -56,12 +55,6 @@ const messageActions = {
if (message.args.thought) {
store.dispatch(addAssistantMessage(message.args.thought));
}
if (
!message.args.is_confirmed ||
message.args.is_confirmed !== "rejected"
) {
store.dispatch(appendInput(message.args.command));
}
},
[ActionType.RUN_IPYTHON]: (message: ActionMessage) => {
if (message.args.thought) {
Expand Down
4 changes: 3 additions & 1 deletion openhands/agenthub/codeact_agent/codeact_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ def get_observation_message(self, obs: Observation) -> Message | None:
max_message_chars = self.llm.config.max_message_chars
obs_prefix = 'OBSERVATION:\n'
if isinstance(obs, CmdOutputObservation):
text = obs_prefix + truncate_content(obs.content, max_message_chars)
text = obs_prefix + truncate_content(
obs.content + obs.interpreter_details, max_message_chars
)
text += (
f'\n[Command {obs.command_id} finished with exit code {obs.exit_code}]'
)
Expand Down
4 changes: 3 additions & 1 deletion openhands/agenthub/codeact_swe_agent/codeact_swe_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ def get_action_message(self, action: Action) -> Message | None:
def get_observation_message(self, obs: Observation) -> Message | None:
max_message_chars = self.llm.config.max_message_chars
if isinstance(obs, CmdOutputObservation):
text = 'OBSERVATION:\n' + truncate_content(obs.content, max_message_chars)
text = 'OBSERVATION:\n' + truncate_content(
obs.content + obs.interpreter_details, max_message_chars
)
text += (
f'\n[Command {obs.command_id} finished with exit code {obs.exit_code}]'
)
Expand Down
6 changes: 2 additions & 4 deletions openhands/controller/agent_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,7 @@ async def _handle_observation(self, observation: Observation):
"""
if (
self._pending_action
and hasattr(self._pending_action, 'is_confirmed')
and self._pending_action.is_confirmed
and getattr(self._pending_action, 'is_confirmed', None)
== ActionConfirmationStatus.AWAITING_CONFIRMATION
):
return
Expand Down Expand Up @@ -458,8 +457,7 @@ async def _step(self) -> None:

if not isinstance(action, NullAction):
if (
hasattr(action, 'is_confirmed')
and action.is_confirmed
getattr(action, 'is_confirmed', None)
== ActionConfirmationStatus.AWAITING_CONFIRMATION
):
await self.set_agent_state_to(AgentState.AWAITING_USER_CONFIRMATION)
Expand Down
1 change: 1 addition & 0 deletions openhands/events/observation/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class CmdOutputObservation(Observation):
exit_code: int = 0
hidden: bool = False
observation: str = ObservationType.RUN
interpreter_details: str = ''

@property
def error(self) -> bool:
Expand Down
22 changes: 17 additions & 5 deletions openhands/runtime/action_execution_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
FileWriteAction,
IPythonRunCellAction,
)
from openhands.events.event import EventSource
from openhands.events.observation import (
CmdOutputObservation,
ErrorObservation,
Expand Down Expand Up @@ -434,6 +435,7 @@ async def run(self, action: CmdRunAction) -> CmdOutputObservation:
), f'Timeout argument is required for CmdRunAction: {action}'
commands = split_bash_commands(action.command)
all_output = ''
python_interpreter = ''
for command in commands:
if command == '':
output, exit_code = self._continue_bash(
Expand All @@ -454,21 +456,31 @@ async def run(self, action: CmdRunAction) -> CmdOutputObservation:
keep_prompt=action.keep_prompt,
kill_on_timeout=False if not action.blocking else True,
)
# Get rid of the python interpreter string from each line of the output.
# We need it only once at the end.
parts = output.rsplit('[Python Interpreter: ', 1)
output = parts[0]
if len(parts) == 2:
python_interpreter = '[Python Interpreter: ' + parts[1]
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'
# previous output already exists so we add a newline
all_output += '\r\n'

all_output += str(output) + '\r\n'
# If the command originated with the agent, append the command that was run...
if action.source == EventSource.AGENT:
all_output += command + '\r\n'

all_output += str(output)
if exit_code != 0:
break

return CmdOutputObservation(
command_id=-1,
content=all_output.rstrip('\r\n'),
command=action.command,
hidden=action.hidden,
exit_code=exit_code,
interpreter_details=python_interpreter,
)
except UnicodeDecodeError:
raise RuntimeError('Command output could not be decoded as utf-8')
Expand Down
17 changes: 8 additions & 9 deletions tests/runtime/test_bash.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def test_bash_timeout_and_keyboard_interrupt(temp_dir, box_class, run_as_openhan
obs = runtime.run_action(action)
assert isinstance(obs, CmdOutputObservation)
assert obs.exit_code == 0
assert '/workspace' in obs.content
assert '/workspace' in obs.interpreter_details

finally:
_close_test_runtime(runtime)
Expand All @@ -121,7 +121,7 @@ def test_bash_pexcept_eof(temp_dir, box_class, run_as_openhands):
logger.info(obs, extra={'msg_type': 'OBSERVATION'})
assert isinstance(obs, CmdOutputObservation)
assert obs.exit_code == 0
assert '/workspace' in obs.content
assert '/workspace' in obs.interpreter_details

# run it again!
action = CmdRunAction(command='python3 -m http.server 8080')
Expand All @@ -139,7 +139,7 @@ def test_bash_pexcept_eof(temp_dir, box_class, run_as_openhands):
obs = runtime.run_action(action)
assert isinstance(obs, CmdOutputObservation)
assert obs.exit_code == 0
assert '/workspace' in obs.content
assert '/workspace' in obs.interpreter_details
finally:
_close_test_runtime(runtime)

Expand Down Expand Up @@ -190,7 +190,7 @@ def test_process_resistant_to_one_sigint(temp_dir, box_class, run_as_openhands):
obs = runtime.run_action(action)
assert isinstance(obs, CmdOutputObservation)
assert obs.exit_code == 0
assert '/workspace' in obs.content
assert '/workspace' in obs.interpreter_details
assert 'resistant_script.sh' in obs.content

finally:
Expand Down Expand Up @@ -243,7 +243,7 @@ def test_process_resistant_to_multiple_sigint(temp_dir, box_class, run_as_openha
obs = runtime.run_action(action)
assert isinstance(obs, CmdOutputObservation)
assert obs.exit_code == 0
assert '/workspace' in obs.content
assert '/workspace' in obs.interpreter_details
assert 'resistant_script.sh' in obs.content

finally:
Expand All @@ -264,7 +264,7 @@ def test_multiline_commands(temp_dir, box_class):
assert 'hello\r\nworld' in obs.content

# test whitespace
obs = _run_cmd_action(runtime, 'echo -e "\\n\\n\\n"')
obs = _run_cmd_action(runtime, 'echo -e "a\\n\\n\\nz"')
assert obs.exit_code == 0, 'The exit code should be 0.'
assert '\r\n\r\n\r\n' in obs.content
finally:
Expand Down Expand Up @@ -312,7 +312,6 @@ def test_multiple_multiline_commands(temp_dir, box_class, run_as_openhands):
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
finally:
_close_test_runtime(runtime)

Expand Down Expand Up @@ -604,13 +603,13 @@ def test_keep_prompt(box_class, temp_dir):

obs = _run_cmd_action(runtime, f'touch {sandbox_dir}/test_file.txt')
assert obs.exit_code == 0
assert 'root@' in obs.content
assert 'root@' in obs.interpreter_details

obs = _run_cmd_action(
runtime, f'cat {sandbox_dir}/test_file.txt', keep_prompt=False
)
assert obs.exit_code == 0
assert 'root@' not in obs.content
assert 'root@' not in obs.interpreter_details
finally:
_close_test_runtime(runtime)

Expand Down
2 changes: 2 additions & 0 deletions tests/unit/test_observation_serialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def test_observation_event_props_serialization_deserialization():
'command': 'ls -l',
'command_id': 3,
'hidden': False,
'interpreter_details': '',
},
'content': 'foo.txt',
}
Expand All @@ -66,6 +67,7 @@ def test_command_output_observation_serialization_deserialization():
'command': 'ls -l',
'command_id': 3,
'hidden': False,
'interpreter_details': '',
},
'message': 'Command `ls -l` executed with exit code 0.',
'content': 'foo.txt',
Expand Down

0 comments on commit 90e2bf4

Please sign in to comment.