OpenHands icon indicating copy to clipboard operation
OpenHands copied to clipboard

Add username parameter to AsyncBashSession

Open rbren opened this issue 7 months ago • 6 comments

  • [ ] 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:

  1. Added a username parameter to AsyncBashSession.execute method
  2. Added logic to handle the username parameter in AsyncBashSession, following the pattern in BashSession
  3. 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

rbren avatar May 27 '25 22:05 rbren

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-staging[bot] avatar May 27 '25 23:05 openhands-staging[bot]

@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

rbren avatar May 28 '25 12:05 rbren

I'm on it! rbren can track my progress at all-hands.dev

openhands-ai[bot] avatar May 28 '25 12:05 openhands-ai[bot]

@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) =============

rbren avatar May 28 '25 13:05 rbren

I'm on it! rbren can track my progress at all-hands.dev

openhands-ai[bot] avatar May 28 '25 13:05 openhands-ai[bot]

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:

  1. test_clone_or_init_repo_no_repo_with_user_id
  2. test_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".

openhands-ai[bot] avatar May 28 '25 13:05 openhands-ai[bot]