Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split bash commands by the new line character #4462

Merged
merged 30 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
b36d7bc
Fix for split commands into multiple around new lines
tofarr Oct 17, 2024
5b62bff
Handle empty commands
tofarr Oct 17, 2024
a7998d6
Merge branch 'main' into feat_split_commands_by_new_line
tofarr Oct 17, 2024
061400f
Lint fixes
tofarr Oct 17, 2024
daf4f32
Used existing split command
tofarr Oct 18, 2024
418db23
Merge branch 'main' into feat_split_commands_by_new_line
tofarr Oct 21, 2024
8bce4ac
Added comment
tofarr Oct 21, 2024
2c72e32
Now works with confirmation mode
tofarr Oct 21, 2024
e985e5c
Merge branch 'main' into feat_split_commands_by_new_line
tofarr Oct 21, 2024
cbf5490
Fix agent running bug
tofarr Oct 21, 2024
8cc70dc
Merge branch 'feat_split_commands_by_new_line' of github.com:All-Hand…
tofarr Oct 21, 2024
e4df45b
Merge branch 'main' into feat_split_commands_by_new_line
tofarr Oct 22, 2024
5cbab76
WIP
tofarr Oct 22, 2024
e2ba150
WIP
tofarr Oct 22, 2024
0152e2f
WIP
tofarr Oct 22, 2024
e34ce2a
Messages are only added on completion
tofarr Oct 22, 2024
13a6890
Merge branch 'main' into feat_split_commands_by_new_line
tofarr Oct 23, 2024
106e20c
Lint fix
tofarr Oct 23, 2024
378056b
Merge branch 'main' into feat_split_commands_by_new_line
tofarr Oct 23, 2024
7999c6a
Merge branch 'main' into feat_split_commands_by_new_line
tofarr Oct 23, 2024
d061c9d
Added python interpreter at end
tofarr Oct 23, 2024
dd81837
First refactor
tofarr Oct 23, 2024
499833d
Test fix
tofarr Oct 23, 2024
2c66288
Reinforced
tofarr Oct 23, 2024
b89c88d
Fix tests
tofarr Oct 23, 2024
7fd0036
Merge branch 'main' into feat_split_commands_by_new_line
tofarr Oct 23, 2024
3946749
Updated name as requested
tofarr Oct 23, 2024
32b4f92
Reverted
tofarr Oct 23, 2024
bcd496a
Merge branch 'main' into feat_split_commands_by_new_line
tofarr Oct 23, 2024
b5294d1
Test fix
tofarr Oct 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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));
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commands are only outputted on completion

},
[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