mirror of
https://github.com/All-Hands-AI/OpenHands.git
synced 2026-01-09 14:57:59 -05:00
refactor(git): principled way to set git configuration for agents & re-enable git settings in UI (#10293)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
@@ -249,11 +249,11 @@ function AppSettingsScreen() {
|
||||
className="w-full max-w-[680px]" // Match the width of the language field
|
||||
/>
|
||||
|
||||
<div className="border-t border-t-tertiary pt-6 mt-2 hidden">
|
||||
<div className="border-t border-t-tertiary pt-6 mt-2">
|
||||
<h3 className="text-lg font-medium mb-2">
|
||||
{t(I18nKey.SETTINGS$GIT_SETTINGS)}
|
||||
</h3>
|
||||
<p className="text-sm text-secondary mb-4">
|
||||
<p className="text-xs mb-4">
|
||||
{t(I18nKey.SETTINGS$GIT_SETTINGS_DESCRIPTION)}
|
||||
</p>
|
||||
<div className="flex flex-col gap-6">
|
||||
|
||||
@@ -723,9 +723,6 @@ def run_cli_command(args):
|
||||
except ConnectionRefusedError as e:
|
||||
print_formatted_text(f'Connection refused: {e}')
|
||||
sys.exit(1)
|
||||
except Exception as e:
|
||||
print_formatted_text(f'An error occurred: {e}')
|
||||
sys.exit(1)
|
||||
finally:
|
||||
try:
|
||||
# Cancel all running tasks
|
||||
|
||||
@@ -176,15 +176,11 @@ class ActionExecutor:
|
||||
user_id: int,
|
||||
enable_browser: bool,
|
||||
browsergym_eval_env: str | None,
|
||||
git_user_name: str = 'openhands',
|
||||
git_user_email: str = 'openhands@all-hands.dev',
|
||||
) -> None:
|
||||
self.plugins_to_load = plugins_to_load
|
||||
self._initial_cwd = work_dir
|
||||
self.username = username
|
||||
self.user_id = user_id
|
||||
self.git_user_name = git_user_name
|
||||
self.git_user_email = git_user_email
|
||||
_updated_user_id = init_user_and_working_directory(
|
||||
username=username, user_id=self.user_id, initial_cwd=work_dir
|
||||
)
|
||||
@@ -344,40 +340,11 @@ class ActionExecutor:
|
||||
)
|
||||
|
||||
async def _init_bash_commands(self):
|
||||
# You can add any bash commands you want to run on startup here
|
||||
# It is empty because: Git configuration is now handled by the runtime client after connection
|
||||
INIT_COMMANDS = []
|
||||
is_local_runtime = os.environ.get('LOCAL_RUNTIME_MODE') == '1'
|
||||
is_windows = sys.platform == 'win32'
|
||||
|
||||
# Determine git config commands based on platform and runtime mode
|
||||
if is_local_runtime:
|
||||
if is_windows:
|
||||
# Windows, local - split into separate commands
|
||||
INIT_COMMANDS.append(
|
||||
f'git config --file ./.git_config user.name "{self.git_user_name}"'
|
||||
)
|
||||
INIT_COMMANDS.append(
|
||||
f'git config --file ./.git_config user.email "{self.git_user_email}"'
|
||||
)
|
||||
INIT_COMMANDS.append(
|
||||
'$env:GIT_CONFIG = (Join-Path (Get-Location) ".git_config")'
|
||||
)
|
||||
else:
|
||||
INIT_COMMANDS.append(
|
||||
f'git config --file ./.git_config user.name "{self.git_user_name}"'
|
||||
)
|
||||
INIT_COMMANDS.append(
|
||||
f'git config --file ./.git_config user.email "{self.git_user_email}"'
|
||||
)
|
||||
INIT_COMMANDS.append('export GIT_CONFIG=$(pwd)/.git_config')
|
||||
else:
|
||||
# Non-local (implies Linux/macOS)
|
||||
INIT_COMMANDS.append(
|
||||
f'git config --global user.name "{self.git_user_name}"'
|
||||
)
|
||||
INIT_COMMANDS.append(
|
||||
f'git config --global user.email "{self.git_user_email}"'
|
||||
)
|
||||
|
||||
# Determine no-pager command
|
||||
if is_windows:
|
||||
no_pager_cmd = 'function git { git.exe --no-pager $args }'
|
||||
@@ -696,18 +663,6 @@ if __name__ == '__main__':
|
||||
help='BrowserGym environment used for browser evaluation',
|
||||
default=None,
|
||||
)
|
||||
parser.add_argument(
|
||||
'--git-user-name',
|
||||
type=str,
|
||||
help='Git user name for commits',
|
||||
default='openhands',
|
||||
)
|
||||
parser.add_argument(
|
||||
'--git-user-email',
|
||||
type=str,
|
||||
help='Git user email for commits',
|
||||
default='openhands@all-hands.dev',
|
||||
)
|
||||
|
||||
# example: python client.py 8000 --working-dir /workspace --plugins JupyterRequirement
|
||||
args = parser.parse_args()
|
||||
@@ -741,8 +696,6 @@ if __name__ == '__main__':
|
||||
user_id=args.user_id,
|
||||
enable_browser=args.enable_browser,
|
||||
browsergym_eval_env=args.browsergym_eval_env,
|
||||
git_user_name=args.git_user_name,
|
||||
git_user_email=args.git_user_email,
|
||||
)
|
||||
await client.ainit()
|
||||
logger.info('ActionExecutor initialized.')
|
||||
|
||||
@@ -195,6 +195,9 @@ class Runtime(FileEditRuntimeMixin):
|
||||
if self.config.sandbox.runtime_startup_env_vars:
|
||||
self.add_env_vars(self.config.sandbox.runtime_startup_env_vars)
|
||||
|
||||
# Configure git settings
|
||||
self._setup_git_config()
|
||||
|
||||
def close(self) -> None:
|
||||
"""This should only be called by conversation manager or closing the session.
|
||||
If called for instance by error handling, it could prevent recovery.
|
||||
@@ -904,6 +907,42 @@ fi
|
||||
async def connect(self) -> None:
|
||||
pass
|
||||
|
||||
def _setup_git_config(self) -> None:
|
||||
"""Configure git user settings during initial environment setup.
|
||||
|
||||
This method is called automatically during setup_initial_env() to ensure
|
||||
git configuration is applied to the runtime environment.
|
||||
"""
|
||||
# Get git configuration from config
|
||||
git_user_name = self.config.git_user_name
|
||||
git_user_email = self.config.git_user_email
|
||||
|
||||
# Skip git configuration for CLI runtime to preserve user's local git settings
|
||||
is_cli_runtime = self.config.runtime == 'cli'
|
||||
if is_cli_runtime:
|
||||
logger.debug(
|
||||
"Skipping git configuration for CLI runtime - using user's local git config"
|
||||
)
|
||||
return
|
||||
|
||||
# All runtimes (except CLI) use global git config
|
||||
cmd = f'git config --global user.name "{git_user_name}" && git config --global user.email "{git_user_email}"'
|
||||
|
||||
# Execute git configuration command
|
||||
try:
|
||||
action = CmdRunAction(command=cmd)
|
||||
obs = self.run(action)
|
||||
if isinstance(obs, CmdOutputObservation) and obs.exit_code != 0:
|
||||
logger.warning(
|
||||
f'Git config command failed: {cmd}, error: {obs.content}'
|
||||
)
|
||||
else:
|
||||
logger.info(
|
||||
f'Successfully configured git: name={git_user_name}, email={git_user_email}'
|
||||
)
|
||||
except Exception as e:
|
||||
logger.warning(f'Failed to execute git config command: {cmd}, error: {e}')
|
||||
|
||||
@abstractmethod
|
||||
def get_mcp_config(
|
||||
self, extra_stdio_servers: list[MCPStdioServerConfig] | None = None
|
||||
|
||||
@@ -57,10 +57,6 @@ def get_action_execution_server_startup_command(
|
||||
username,
|
||||
'--user-id',
|
||||
str(user_id),
|
||||
'--git-user-name',
|
||||
app_config.git_user_name,
|
||||
'--git-user-email',
|
||||
app_config.git_user_email,
|
||||
*browsergym_args,
|
||||
]
|
||||
|
||||
|
||||
@@ -162,6 +162,22 @@ async def store_settings(
|
||||
settings.remote_runtime_resource_factor
|
||||
)
|
||||
|
||||
# Update git configuration with new settings
|
||||
git_config_updated = False
|
||||
if settings.git_user_name is not None:
|
||||
config.git_user_name = settings.git_user_name
|
||||
git_config_updated = True
|
||||
if settings.git_user_email is not None:
|
||||
config.git_user_email = settings.git_user_email
|
||||
git_config_updated = True
|
||||
|
||||
# Note: Git configuration will be applied when new sessions are initialized
|
||||
# Existing sessions will continue with their current git configuration
|
||||
if git_config_updated:
|
||||
logger.info(
|
||||
f'Updated global git configuration: name={config.git_user_name}, email={config.git_user_email}'
|
||||
)
|
||||
|
||||
settings = convert_to_settings(settings)
|
||||
await settings_store.store(settings)
|
||||
return JSONResponse(
|
||||
|
||||
@@ -345,7 +345,8 @@ def test_multiple_multiline_commands(temp_dir, runtime_cls, run_as_openhands):
|
||||
|
||||
# Verify all expected outputs are present
|
||||
if is_windows():
|
||||
assert '.git_config' in results[0] # Get-ChildItem
|
||||
# Get-ChildItem should execute successfully (no specific content check needed)
|
||||
pass # results[0] contains directory listing output
|
||||
else:
|
||||
assert 'total 0' in results[0] # ls -l
|
||||
assert 'hello\nworld' in results[1] # echo -e "hello\nworld"
|
||||
@@ -518,7 +519,7 @@ def test_multi_cmd_run_in_single_line(temp_dir, runtime_cls):
|
||||
obs = _run_cmd_action(runtime, 'Get-Location && Get-ChildItem')
|
||||
assert obs.exit_code == 0
|
||||
assert config.workspace_mount_path_in_sandbox in obs.content
|
||||
assert '.git_config' in obs.content
|
||||
# Git config is now handled by runtime base class, not as a file
|
||||
else:
|
||||
# Original Linux version using &&
|
||||
obs = _run_cmd_action(runtime, 'pwd && ls -l')
|
||||
|
||||
@@ -28,8 +28,12 @@ class TestGitConfig:
|
||||
assert config.git_user_name == 'testuser'
|
||||
assert config.git_user_email == 'testuser@example.com'
|
||||
|
||||
def test_git_config_in_command_generation(self):
|
||||
"""Test that git configuration is properly passed to action execution server command."""
|
||||
def test_git_config_not_in_command_generation(self):
|
||||
"""Test that git configuration is NOT passed as command line arguments.
|
||||
|
||||
Git configuration is handled by the runtime base class via git config commands,
|
||||
not through command line arguments to the action execution server.
|
||||
"""
|
||||
config = OpenHandsConfig()
|
||||
config.git_user_name = 'customuser'
|
||||
config.git_user_email = 'customuser@example.com'
|
||||
@@ -42,14 +46,19 @@ class TestGitConfig:
|
||||
python_executable='python',
|
||||
)
|
||||
|
||||
# Check that git config arguments are in the command
|
||||
assert '--git-user-name' in cmd
|
||||
assert 'customuser' in cmd
|
||||
assert '--git-user-email' in cmd
|
||||
assert 'customuser@example.com' in cmd
|
||||
# Check that git config arguments are NOT in the command
|
||||
assert '--git-user-name' not in cmd
|
||||
assert '--git-user-email' not in cmd
|
||||
# The git config values themselves should also not be in the command
|
||||
assert 'customuser' not in cmd
|
||||
assert 'customuser@example.com' not in cmd
|
||||
|
||||
def test_git_config_with_special_characters(self):
|
||||
"""Test that git configuration handles special characters correctly."""
|
||||
def test_git_config_with_special_characters_not_in_command(self):
|
||||
"""Test that git configuration with special characters is NOT in command.
|
||||
|
||||
Git configuration is handled by the runtime base class via git config commands,
|
||||
not through command line arguments to the action execution server.
|
||||
"""
|
||||
config = OpenHandsConfig()
|
||||
config.git_user_name = 'User With Spaces'
|
||||
config.git_user_email = 'user+tag@example.com'
|
||||
@@ -62,8 +71,9 @@ class TestGitConfig:
|
||||
python_executable='python',
|
||||
)
|
||||
|
||||
assert 'User With Spaces' in cmd
|
||||
assert 'user+tag@example.com' in cmd
|
||||
# Git config values should NOT be in the command
|
||||
assert 'User With Spaces' not in cmd
|
||||
assert 'user+tag@example.com' not in cmd
|
||||
|
||||
def test_git_config_empty_values(self):
|
||||
"""Test behavior with empty git configuration values."""
|
||||
|
||||
Reference in New Issue
Block a user