github/OpenHands
1
1
Fork 0
mirror of https://github.com/All-Hands-AI/OpenHands.git synced 2026-01-09 14:57:59 -05:00
Code Issues Packages Projects Releases Wiki Activity

Fix for running git commands with the proper user (#8898)

Browse Source
This commit is contained in:
Robert Brennan
2025-06-05 17:20:15 -07:00
committed by GitHub
parent 19fcf427ba
commit 0813c113f0
10 changed files with 134 additions and 129 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
.github/workflows/ghcr-build.yml vendored
View File

@@ -313,6 +313,8 @@ jobs:
TEST_IN_CI=true \
RUN_AS_OPENHANDS=false \
poetry run pytest -n 7 -raRs --reruns 2 --reruns-delay 5 -s ./tests/runtime --ignore=tests/runtime/test_browsergym_envs.py --durations=10
env:
DEBUG: "1"
# Run unit tests with the Docker runtime Docker images as openhands user
test_runtime_oh:
@@ -378,6 +380,8 @@ jobs:
TEST_IN_CI=true \
RUN_AS_OPENHANDS=true \
poetry run pytest -n 7 -raRs --reruns 2 --reruns-delay 5 -s ./tests/runtime --ignore=tests/runtime/test_browsergym_envs.py --durations=10
env:
DEBUG: "1"
# The two following jobs (named identically) are to check whether all the runtime tests have passed as the
# "All Runtime Tests Passed" is a required job for PRs to merge

6
.github/workflows/py-unit-tests.yml vendored
View File

@@ -74,5 +74,11 @@ jobs:
run: poetry install --with dev,test,runtime
- name: Run Windows unit tests
run: poetry run pytest -svv tests/unit/test_windows_bash.py
env:
DEBUG: "1"
- name: Run Windows runtime tests with LocalRuntime
run: $env:TEST_RUNTIME="local"; poetry run pytest -svv tests/runtime/test_bash.py
env:
TEST_RUNTIME: local
DEBUG: "1"

2
frontend/src/components/features/diff-viewer/file-diff-viewer.tsx
View File

@@ -122,7 +122,7 @@ export function FileDiffViewer({ path, type }: FileDiffViewerProps) {
modifiedEditor.onDidContentSizeChange(updateEditorHeight);
};
const status = type === "U" ? STATUS_MAP.A : STATUS_MAP[type];
const status = (type === "U" ? STATUS_MAP.A : STATUS_MAP[type]) || "?";
let statusIcon: React.ReactNode;
if (typeof status === "string") {

36
openhands/runtime/action_execution_server.py
View File

@@ -65,7 +65,6 @@ from openhands.runtime.browser.browser_env import BrowserEnv
from openhands.runtime.file_viewer_server import start_file_viewer_server
from openhands.runtime.plugins import ALL_PLUGINS, JupyterPlugin, Plugin, VSCodePlugin
from openhands.runtime.utils import find_available_tcp_port
from openhands.runtime.utils.async_bash import AsyncBashSession
from openhands.runtime.utils.bash import BashSession
from openhands.runtime.utils.files import insert_lines, read_lines
from openhands.runtime.utils.log_capture import capture_logs
@@ -254,12 +253,10 @@ class ActionExecutor:
# If we get here, the browser is ready
logger.debug('Browser is ready')
async def ainit(self):
# bash needs to be initialized first
logger.debug('Initializing bash session')
def _create_bash_session(self, cwd: str | None = None):
if sys.platform == 'win32':
self.bash_session = WindowsPowershellSession( # type: ignore[name-defined]
work_dir=self._initial_cwd,
return WindowsPowershellSession( # type: ignore[name-defined]
work_dir=cwd or self._initial_cwd,
username=self.username,
no_change_timeout_seconds=int(
os.environ.get('NO_CHANGE_TIMEOUT_SECONDS', 10)
@@ -267,15 +264,21 @@ class ActionExecutor:
max_memory_mb=self.max_memory_gb * 1024 if self.max_memory_gb else None,
)
else:
self.bash_session = BashSession(
work_dir=self._initial_cwd,
bash_session = BashSession(
work_dir=cwd or self._initial_cwd,
username=self.username,
no_change_timeout_seconds=int(
os.environ.get('NO_CHANGE_TIMEOUT_SECONDS', 10)
),
max_memory_mb=self.max_memory_gb * 1024 if self.max_memory_gb else None,
)
self.bash_session.initialize()
bash_session.initialize()
return bash_session
async def ainit(self):
# bash needs to be initialized first
logger.debug('Initializing bash session')
self.bash_session = self._create_bash_session()
logger.debug('Bash session initialized')
# Start browser initialization in the background
@@ -388,18 +391,11 @@ class ActionExecutor:
self, action: CmdRunAction
) -> CmdOutputObservation | ErrorObservation:
try:
bash_session = self.bash_session
if action.is_static:
path = action.cwd or self._initial_cwd
result = await AsyncBashSession.execute(action.command, path)
obs = CmdOutputObservation(
content=result.content,
exit_code=result.exit_code,
command=action.command,
)
return obs
assert self.bash_session is not None
obs = await call_sync_from_async(self.bash_session.execute, action)
bash_session = self._create_bash_session(action.cwd)
assert bash_session is not None
obs = await call_sync_from_async(bash_session.execute, action)
return obs
except Exception as e:
logger.error(f'Error running command: {e}')

5
openhands/runtime/base.py
View File

@@ -400,7 +400,7 @@ class Runtime(FileEditRuntimeMixin):
'No repository selected. Initializing a new git repository in the workspace.'
)
action = CmdRunAction(
command='git init',
command=f'git init && git config --global --add safe.directory {self.workspace_root}'
)
self.run_action(action)
else:
@@ -952,6 +952,9 @@ fi
exit_code = 0
content = ''
if isinstance(obs, ErrorObservation):
exit_code = -1
if hasattr(obs, 'exit_code'):
exit_code = obs.exit_code
if hasattr(obs, 'content'):

6
openhands/runtime/impl/action_execution/action_execution_client.py
View File

@@ -406,7 +406,7 @@ class ActionExecutionClient(Runtime):
'POST',
f'{self.action_execution_server_url}/update_mcp_server',
json=stdio_tools,
timeout=10,
timeout=60,
)
result = response.json()
if response.status_code != 200:
@@ -464,7 +464,9 @@ class ActionExecutionClient(Runtime):
)
# Create clients for this specific operation
mcp_clients = await create_mcp_clients(updated_mcp_config.sse_servers, updated_mcp_config.shttp_servers, self.sid)
mcp_clients = await create_mcp_clients(
updated_mcp_config.sse_servers, updated_mcp_config.shttp_servers, self.sid
)
# Call the tool and return the result
# No need for try/finally since disconnect() is now just resetting state

54
openhands/runtime/utils/async_bash.py
View File

@@ -1,54 +0,0 @@
import asyncio
import os
from openhands.runtime.base import CommandResult
class AsyncBashSession:
@staticmethod
async def execute(command: str, work_dir: str) -> CommandResult:
"""Execute a command in the bash session asynchronously."""
work_dir = os.path.abspath(work_dir)
if not os.path.exists(work_dir):
raise ValueError(f'Work directory {work_dir} does not exist.')
command = command.strip()
if not command:
return CommandResult(content='', exit_code=0)
try:
process = await asyncio.subprocess.create_subprocess_shell(
command,
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE,
cwd=work_dir,
)
try:
stdout, stderr = await asyncio.wait_for(
process.communicate(), timeout=30
)
output = stdout.decode('utf-8')
if stderr:
output = stderr.decode('utf-8')
print(f'!##! Error running command: {stderr.decode("utf-8")}')
return CommandResult(content=output, exit_code=process.returncode or 0)
except asyncio.TimeoutError:
process.terminate()
# Allow a brief moment for cleanup
try:
await asyncio.wait_for(process.wait(), timeout=1.0)
except asyncio.TimeoutError:
process.kill() # Force kill if it doesn't terminate cleanly
return CommandResult(content='Command timed out.', exit_code=-1)
except Exception as e:
return CommandResult(
content=f'Error running command: {str(e)}', exit_code=-1
)

22
openhands/runtime/utils/git_handler.py
View File

@@ -44,7 +44,7 @@ class GitHandler:
Returns:
bool: True if inside a Git repository, otherwise False.
"""
cmd = 'git rev-parse --is-inside-work-tree'
cmd = 'git --no-pager rev-parse --is-inside-work-tree'
output = self.execute(cmd, self.cwd)
return output.content.strip() == 'true'
@@ -71,7 +71,7 @@ class GitHandler:
Returns:
bool: True if the reference exists, otherwise False.
"""
cmd = f'git rev-parse --verify {ref}'
cmd = f'git --no-pager rev-parse --verify {ref}'
output = self.execute(cmd, self.cwd)
return output.exit_code == 0
@@ -86,9 +86,9 @@ class GitHandler:
default_branch = self._get_default_branch()
ref_current_branch = f'origin/{current_branch}'
ref_non_default_branch = f'$(git merge-base HEAD "$(git rev-parse --abbrev-ref origin/{default_branch})")'
ref_non_default_branch = f'$(git --no-pager merge-base HEAD "$(git --no-pager rev-parse --abbrev-ref origin/{default_branch})")'
ref_default_branch = 'origin/' + default_branch
ref_new_repo = '$(git rev-parse --verify 4b825dc642cb6eb9a060e54bf8d69288fbee4904)' # compares with empty tree
ref_new_repo = '$(git --no-pager rev-parse --verify 4b825dc642cb6eb9a060e54bf8d69288fbee4904)' # compares with empty tree
refs = [
ref_current_branch,
@@ -116,7 +116,7 @@ class GitHandler:
if not ref:
return ''
cmd = f'git show {ref}:{file_path}'
cmd = f'git --no-pager show {ref}:{file_path}'
output = self.execute(cmd, self.cwd)
return output.content if output.exit_code == 0 else ''
@@ -127,7 +127,7 @@ class GitHandler:
Returns:
str: The name of the primary branch.
"""
cmd = 'git remote show origin | grep "HEAD branch"'
cmd = 'git --no-pager remote show origin | grep "HEAD branch"'
output = self.execute(cmd, self.cwd)
return output.content.split()[-1].strip()
@@ -138,7 +138,7 @@ class GitHandler:
Returns:
str: The name of the current branch.
"""
cmd = 'git rev-parse --abbrev-ref HEAD'
cmd = 'git --no-pager rev-parse --abbrev-ref HEAD'
output = self.execute(cmd, self.cwd)
return output.content.strip()
@@ -153,8 +153,12 @@ class GitHandler:
if not ref:
return []
diff_cmd = f'git diff --name-status {ref}'
diff_cmd = f'git --no-pager diff --name-status {ref}'
output = self.execute(diff_cmd, self.cwd)
if output.exit_code != 0:
raise RuntimeError(
f'Failed to get diff for ref {ref} in {self.cwd}. Command output: {output.content}'
)
return output.content.splitlines()
def _get_untracked_files(self) -> list[dict[str, str]]:
@@ -164,7 +168,7 @@ class GitHandler:
Returns:
list[dict[str, str]]: A list of dictionaries containing file paths and statuses.
"""
cmd = 'git ls-files --others --exclude-standard'
cmd = 'git --no-pager ls-files --others --exclude-standard'
output = self.execute(cmd, self.cwd)
obs_list = output.content.splitlines()
return (

118
tests/unit/test_git_handler.py
View File

@@ -46,28 +46,40 @@ class TestGitHandler(unittest.TestCase):
def _setup_git_repos(self):
"""Set up real git repositories for testing."""
# Set up origin repository
self._execute_command('git init --initial-branch=main', self.origin_dir)
self._execute_command(
"git config user.email 'test@example.com'", self.origin_dir
'git --no-pager init --initial-branch=main', self.origin_dir
)
self._execute_command(
"git --no-pager config user.email 'test@example.com'", self.origin_dir
)
self._execute_command(
"git --no-pager config user.name 'Test User'", self.origin_dir
)
self._execute_command("git config user.name 'Test User'", self.origin_dir)
# Create a file and commit it
with open(os.path.join(self.origin_dir, 'file1.txt'), 'w') as f:
f.write('Original content')
self._execute_command('git add file1.txt', self.origin_dir)
self._execute_command("git commit -m 'Initial commit'", self.origin_dir)
self._execute_command('git --no-pager add file1.txt', self.origin_dir)
self._execute_command(
"git --no-pager commit -m 'Initial commit'", self.origin_dir
)
# Clone the origin repository to local
self._execute_command(f'git clone {self.origin_dir} {self.local_dir}')
self._execute_command(
"git config user.email 'test@example.com'", self.local_dir
f'git --no-pager clone {self.origin_dir} {self.local_dir}'
)
self._execute_command(
"git --no-pager config user.email 'test@example.com'", self.local_dir
)
self._execute_command(
"git --no-pager config user.name 'Test User'", self.local_dir
)
self._execute_command("git config user.name 'Test User'", self.local_dir)
# Create a feature branch in the local repository
self._execute_command('git checkout -b feature-branch', self.local_dir)
self._execute_command(
'git --no-pager checkout -b feature-branch', self.local_dir
)
# Modify a file and create a new file
with open(os.path.join(self.local_dir, 'file1.txt'), 'w') as f:
@@ -77,32 +89,40 @@ class TestGitHandler(unittest.TestCase):
f.write('New file content')
# Add and commit file1.txt changes to create a baseline
self._execute_command('git add file1.txt', self.local_dir)
self._execute_command("git commit -m 'Update file1.txt'", self.local_dir)
self._execute_command('git --no-pager add file1.txt', self.local_dir)
self._execute_command(
"git --no-pager commit -m 'Update file1.txt'", self.local_dir
)
# Add and commit file2.txt, then modify it
self._execute_command('git add file2.txt', self.local_dir)
self._execute_command("git commit -m 'Add file2.txt'", self.local_dir)
self._execute_command('git --no-pager add file2.txt', self.local_dir)
self._execute_command(
"git --no-pager commit -m 'Add file2.txt'", self.local_dir
)
# Modify file2.txt and stage it
with open(os.path.join(self.local_dir, 'file2.txt'), 'w') as f:
f.write('Modified new file content')
self._execute_command('git add file2.txt', self.local_dir)
self._execute_command('git --no-pager add file2.txt', self.local_dir)
# Create a file that will be deleted
with open(os.path.join(self.local_dir, 'file3.txt'), 'w') as f:
f.write('File to be deleted')
self._execute_command('git add file3.txt', self.local_dir)
self._execute_command("git commit -m 'Add file3.txt'", self.local_dir)
self._execute_command('git rm file3.txt', self.local_dir)
self._execute_command('git --no-pager add file3.txt', self.local_dir)
self._execute_command(
"git --no-pager commit -m 'Add file3.txt'", self.local_dir
)
self._execute_command('git --no-pager rm file3.txt', self.local_dir)
# Modify file1.txt again but don't stage it (unstaged change)
with open(os.path.join(self.local_dir, 'file1.txt'), 'w') as f:
f.write('Modified content again')
# Push the feature branch to origin
self._execute_command('git push -u origin feature-branch', self.local_dir)
self._execute_command(
'git --no-pager push -u origin feature-branch', self.local_dir
)
def test_is_git_repo(self):
"""Test that _is_git_repo returns True for a git repository."""
@@ -111,7 +131,7 @@ class TestGitHandler(unittest.TestCase):
# Verify the command was executed
self.assertTrue(
any(
cmd == 'git rev-parse --is-inside-work-tree'
cmd == 'git --no-pager rev-parse --is-inside-work-tree'
for cmd, _ in self.executed_commands
)
)
@@ -124,7 +144,7 @@ class TestGitHandler(unittest.TestCase):
# Verify the command was executed
self.assertTrue(
any(
cmd == 'git remote show origin | grep "HEAD branch"'
cmd == 'git --no-pager remote show origin | grep "HEAD branch"'
for cmd, _ in self.executed_commands
)
)
@@ -133,11 +153,12 @@ class TestGitHandler(unittest.TestCase):
"""Test that _get_current_branch returns the correct branch name."""
branch = self.git_handler._get_current_branch()
self.assertEqual(branch, 'feature-branch')
print('executed commands:', self.executed_commands)
# Verify the command was executed
self.assertTrue(
any(
cmd == 'git rev-parse --abbrev-ref HEAD'
cmd == 'git --no-pager rev-parse --abbrev-ref HEAD'
for cmd, _ in self.executed_commands
)
)
@@ -152,7 +173,7 @@ class TestGitHandler(unittest.TestCase):
verify_commands = [
cmd
for cmd, _ in self.executed_commands
if cmd.startswith('git rev-parse --verify')
if cmd.startswith('git --no-pager rev-parse --verify')
]
# First should check origin/feature-branch (current branch)
@@ -162,13 +183,17 @@ class TestGitHandler(unittest.TestCase):
self.assertEqual(ref, 'origin/feature-branch')
# Verify the ref exists
result = self._execute_command(f'git rev-parse --verify {ref}', self.local_dir)
result = self._execute_command(
f'git --no-pager rev-parse --verify {ref}', self.local_dir
)
self.assertEqual(result.exit_code, 0)
def test_get_valid_ref_without_origin_current_branch(self):
"""Test that _get_valid_ref falls back to default branch when current branch doesn't exist in origin."""
# Create a new branch that doesn't exist in origin
self._execute_command('git checkout -b new-local-branch', self.local_dir)
self._execute_command(
'git --no-pager checkout -b new-local-branch', self.local_dir
)
# Clear the executed commands to start fresh
self.executed_commands = []
@@ -180,7 +205,7 @@ class TestGitHandler(unittest.TestCase):
verify_commands = [
cmd
for cmd, _ in self.executed_commands
if cmd.startswith('git rev-parse --verify')
if cmd.startswith('git --no-pager rev-parse --verify')
]
# Should have tried origin/new-local-branch first (which doesn't exist)
@@ -193,7 +218,9 @@ class TestGitHandler(unittest.TestCase):
self.assertTrue(ref == 'origin/main' or 'merge-base' in ref)
# Verify the ref exists
result = self._execute_command(f'git rev-parse --verify {ref}', self.local_dir)
result = self._execute_command(
f'git --no-pager rev-parse --verify {ref}', self.local_dir
)
self.assertEqual(result.exit_code, 0)
def test_get_valid_ref_without_origin(self):
@@ -203,15 +230,21 @@ class TestGitHandler(unittest.TestCase):
os.makedirs(no_origin_dir, exist_ok=True)
# Initialize git repo without origin
self._execute_command('git init', no_origin_dir)
self._execute_command("git config user.email 'test@example.com'", no_origin_dir)
self._execute_command("git config user.name 'Test User'", no_origin_dir)
self._execute_command('git --no-pager init', no_origin_dir)
self._execute_command(
"git --no-pager config user.email 'test@example.com'", no_origin_dir
)
self._execute_command(
"git --no-pager config user.name 'Test User'", no_origin_dir
)
# Create a file and commit it
with open(os.path.join(no_origin_dir, 'file1.txt'), 'w') as f:
f.write('Content in repo without origin')
self._execute_command('git add file1.txt', no_origin_dir)
self._execute_command("git commit -m 'Initial commit'", no_origin_dir)
self._execute_command('git --no-pager add file1.txt', no_origin_dir)
self._execute_command(
"git --no-pager commit -m 'Initial commit'", no_origin_dir
)
# Create a custom GitHandler with a modified _get_default_branch method for this test
class TestGitHandler(GitHandler):
@@ -234,19 +267,20 @@ class TestGitHandler(unittest.TestCase):
# Verify that git commands were executed
self.assertTrue(
any(
cmd.startswith('git rev-parse --verify')
cmd.startswith('git --no-pager rev-parse --verify')
for cmd, _ in self.executed_commands
)
)
# Should have fallen back to the empty tree ref
self.assertEqual(
ref, '$(git rev-parse --verify 4b825dc642cb6eb9a060e54bf8d69288fbee4904)'
ref,
'$(git --no-pager rev-parse --verify 4b825dc642cb6eb9a060e54bf8d69288fbee4904)',
)
# Verify the ref exists (the empty tree ref always exists)
result = self._execute_command(
'git rev-parse --verify 4b825dc642cb6eb9a060e54bf8d69288fbee4904',
'git --no-pager rev-parse --verify 4b825dc642cb6eb9a060e54bf8d69288fbee4904',
no_origin_dir,
)
self.assertEqual(result.exit_code, 0)
@@ -258,7 +292,9 @@ class TestGitHandler(unittest.TestCase):
# Should have called _get_valid_ref and then git show
show_commands = [
cmd for cmd, _ in self.executed_commands if cmd.startswith('git show')
cmd
for cmd, _ in self.executed_commands
if cmd.startswith('git --no-pager show')
]
self.assertTrue(any('file1.txt' in cmd for cmd in show_commands))
@@ -277,7 +313,7 @@ class TestGitHandler(unittest.TestCase):
# Let's create a new file to ensure it shows up in the diff
with open(os.path.join(self.local_dir, 'new_file.txt'), 'w') as f:
f.write('New file content')
self._execute_command('git add new_file.txt', self.local_dir)
self._execute_command('git --no-pager add new_file.txt', self.local_dir)
files = self.git_handler._get_changed_files()
self.assertTrue(files)
@@ -291,7 +327,9 @@ class TestGitHandler(unittest.TestCase):
# Should have called _get_valid_ref and then git diff
diff_commands = [
cmd for cmd, _ in self.executed_commands if cmd.startswith('git diff')
cmd
for cmd, _ in self.executed_commands
if cmd.startswith('git --no-pager diff')
]
self.assertTrue(diff_commands)
@@ -309,7 +347,7 @@ class TestGitHandler(unittest.TestCase):
# Verify the command was executed
self.assertTrue(
any(
cmd == 'git ls-files --others --exclude-standard'
cmd == 'git --no-pager ls-files --others --exclude-standard'
for cmd, _ in self.executed_commands
)
)
@@ -323,7 +361,7 @@ class TestGitHandler(unittest.TestCase):
# Create a new file and stage it
with open(os.path.join(self.local_dir, 'new_file2.txt'), 'w') as f:
f.write('New file 2 content')
self._execute_command('git add new_file2.txt', self.local_dir)
self._execute_command('git --no-pager add new_file2.txt', self.local_dir)
changes = self.git_handler.get_git_changes()
self.assertIsNotNone(changes)
@@ -353,7 +391,7 @@ class TestGitHandler(unittest.TestCase):
)
self.assertTrue(
any(
'git show' in cmd and 'file1.txt' in cmd
'git --no-pager show' in cmd and 'file1.txt' in cmd
for cmd, _ in self.executed_commands
)
)

10
tests/unit/test_runtime_git_tokens.py
View File

@@ -234,7 +234,10 @@ async def test_clone_or_init_repo_no_repo_with_user_id(temp_dir):
# 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'
assert (
runtime.run_action_calls[0].command
== f'git init && git config --global --add safe.directory {runtime.workspace_root}'
)
assert result == ''
@@ -255,7 +258,10 @@ async def test_clone_or_init_repo_no_repo_no_user_id_no_workspace_base(temp_dir)
# 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'
assert (
runtime.run_action_calls[0].command
== f'git init && git config --global --add safe.directory {runtime.workspace_root}'
)
assert result == ''