Add username parameter to AsyncBashSession
- [ ] This change is worth documenting at https://docs.all-hands.dev/
- [ ] Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below
End-user friendly description of the problem this fixes or functionality this introduces.
Adds username handling to AsyncBashSession, similar to how BashSession handles usernames.
Summarize what the PR does, explaining any non-trivial design decisions.
This PR modifies the AsyncBashSession class to take in a username parameter and handle it in a way that is consistent with BashSession. It also updates the action execution server to pass the correct username to AsyncBashSession.
Specifically:
- Added a username parameter to AsyncBashSession.execute method
- Added logic to handle the username parameter in AsyncBashSession, following the pattern in BashSession
- Updated the action execution server to pass the username to AsyncBashSession.execute
Link of any specific issues this addresses:
N/A
To run this PR locally, use the following command:
docker run -it --rm -p 3000:3000 -v /var/run/docker.sock:/var/run/docker.sock --add-host host.docker.internal:host-gateway -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:7908b7a-nikolaik --name openhands-app-7908b7a docker.all-hands.dev/all-hands-ai/openhands:7908b7a
Looks like there are a few issues preventing this PR from being merged!
- GitHub Actions are failing:
- Run Python Unit Tests
If you'd like me to help, just leave a comment, like
@OpenHands please fix the failing actions on PR #8746
Feel free to include any additional details that might help me get this PR into a better state.
You can manage your notification settings
@openhands update these unit tests
=================================== FAILURES ===================================
_________________ test_clone_or_init_repo_no_repo_with_user_id _________________
[gw0] linux -- Python 3.12.4 /home/runner/.cache/pypoetry/virtualenvs/openhands-ai-2f2V0C4o-py3.12/bin/python
temp_dir = '/tmp/pytest-of-runner/pytest-0/popen-gw0/test_event_stream0'
@pytest.mark.asyncio
async def test_clone_or_init_repo_no_repo_with_user_id(temp_dir):
"""Test that git init is run when no repository is selected and user_id is set"""
config = AppConfig()
file_store = get_file_store('local', temp_dir)
event_stream = EventStream('abc', file_store)
runtime = TestRuntime(
config=config, event_stream=event_stream, sid='test', user_id='test_user'
)
# Call the function with no repository
result = await runtime.clone_or_init_repo(None, None, None)
# Verify that git init was called
assert len(runtime.run_action_calls) == 1
assert isinstance(runtime.run_action_calls[0], CmdRunAction)
> assert runtime.run_action_calls[0].command == 'git init'
E AssertionError: assert 'git init && git config --global --add safe.directory /workspace' == 'git init'
E
E - git init
E + git init && git config --global --add safe.directory /workspace
tests/unit/test_runtime_git_tokens.py:237: AssertionError
_________ test_clone_or_init_repo_no_repo_no_user_id_no_workspace_base _________
[gw0] linux -- Python 3.12.4 /home/runner/.cache/pypoetry/virtualenvs/openhands-ai-2f2V0C4o-py3.12/bin/python
temp_dir = '/tmp/pytest-of-runner/pytest-0/popen-gw0/test_event_stream0'
@pytest.mark.asyncio
async def test_clone_or_init_repo_no_repo_no_user_id_no_workspace_base(temp_dir):
"""Test that git init is run when no repository is selected, no user_id, and no workspace_base"""
config = AppConfig()
config.workspace_base = None # Ensure workspace_base is not set
file_store = get_file_store('local', temp_dir)
event_stream = EventStream('abc', file_store)
runtime = TestRuntime(
config=config, event_stream=event_stream, sid='test', user_id=None
)
# Call the function with no repository
result = await runtime.clone_or_init_repo(None, None, None)
# Verify that git init was called
assert len(runtime.run_action_calls) == 1
assert isinstance(runtime.run_action_calls[0], CmdRunAction)
> assert runtime.run_action_calls[0].command == 'git init'
E AssertionError: assert 'git init && git config --global --add safe.directory /workspace' == 'git init'
E
E - git init
E + git init && git config --global --add safe.directory /workspace
tests/unit/test_runtime_git_tokens.py:258: AssertionError
I'm on it! rbren can track my progress at all-hands.dev
@openhands update these tests
=================================== FAILURES ===================================
_________________ test_clone_or_init_repo_no_repo_with_user_id _________________
[gw0] linux -- Python 3.12.4 /home/runner/.cache/pypoetry/virtualenvs/openhands-ai-2f2V0C4o-py3.12/bin/python
temp_dir = '/tmp/pytest-of-runner/pytest-0/popen-gw0/test_event_stream0'
@pytest.mark.asyncio
async def test_clone_or_init_repo_no_repo_with_user_id(temp_dir):
"""Test that git init is run when no repository is selected and user_id is set"""
config = AppConfig()
file_store = get_file_store('local', temp_dir)
event_stream = EventStream('abc', file_store)
runtime = TestRuntime(
config=config, event_stream=event_stream, sid='test', user_id='test_user'
)
# Call the function with no repository
result = await runtime.clone_or_init_repo(None, None, None)
# Verify that git init was called
assert len(runtime.run_action_calls) == 1
assert isinstance(runtime.run_action_calls[0], CmdRunAction)
> assert runtime.run_action_calls[0].command == 'git init'
E AssertionError: assert 'git init && git config --global --add safe.directory /workspace' == 'git init'
E
E - git init
E + git init && git config --global --add safe.directory /workspace
tests/unit/test_runtime_git_tokens.py:237: AssertionError
_________ test_clone_or_init_repo_no_repo_no_user_id_no_workspace_base _________
[gw0] linux -- Python 3.12.4 /home/runner/.cache/pypoetry/virtualenvs/openhands-ai-2f2V0C4o-py3.12/bin/python
temp_dir = '/tmp/pytest-of-runner/pytest-0/popen-gw0/test_event_stream0'
@pytest.mark.asyncio
async def test_clone_or_init_repo_no_repo_no_user_id_no_workspace_base(temp_dir):
"""Test that git init is run when no repository is selected, no user_id, and no workspace_base"""
config = AppConfig()
config.workspace_base = None # Ensure workspace_base is not set
file_store = get_file_store('local', temp_dir)
event_stream = EventStream('abc', file_store)
runtime = TestRuntime(
config=config, event_stream=event_stream, sid='test', user_id=None
)
# Call the function with no repository
result = await runtime.clone_or_init_repo(None, None, None)
# Verify that git init was called
assert len(runtime.run_action_calls) == 1
assert isinstance(runtime.run_action_calls[0], CmdRunAction)
> assert runtime.run_action_calls[0].command == 'git init'
E AssertionError: assert 'git init && git config --global --add safe.directory /workspace' == 'git init'
E
E - git init
E + git init && git config --global --add safe.directory /workspace
tests/unit/test_runtime_git_tokens.py:258: AssertionError
=========================== short test summary info ============================
FAILED tests/unit/test_runtime_git_tokens.py::test_clone_or_init_repo_no_repo_with_user_id
FAILED tests/unit/test_runtime_git_tokens.py::test_clone_or_init_repo_no_repo_no_user_id_no_workspace_base
============ 2 failed, 1137 passed, 23 skipped in 71.33s (0:01:11) =============
I'm on it! rbren can track my progress at all-hands.dev
I've successfully updated the failing tests in the PR #8746 on branch add-username-to-async-bash-session.
Summary of Changes
The PR had two failing tests related to the git init command in the clone_or_init_repo method:
test_clone_or_init_repo_no_repo_with_user_idtest_clone_or_init_repo_no_repo_no_user_id_no_workspace_base
The tests were failing because they expected the command to be just git init, but the actual implementation had been updated to include a git config command to add the workspace directory as a safe directory: git init && git config --global --add safe.directory /workspace.
I updated both test assertions to match the current implementation by changing:
assert runtime.run_action_calls[0].command == 'git init'
to:
assert runtime.run_action_calls[0].command == f'git init && git config --global --add safe.directory {runtime.workspace_root}'
After making these changes, I ran the tests to verify they now pass, and then committed and pushed the changes to the remote branch.
The changes have been successfully pushed to the add-username-to-async-bash-session branch with commit message "Update git init tests to match current implementation with safe.directory config".