From d3983b00bda83af6bd4cb5912d1907b9f9055253 Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Fri, 1 Aug 2025 23:20:05 +0200 Subject: [PATCH] [Feature Request]: Make git username and email configurable (#9942) Co-authored-by: openhands --- .../codeact_agent/prompts/system_prompt.j2 | 2 +- .../prompts/system_prompt_interactive.j2 | 2 +- .../prompts/system_prompt_long_horizon.j2 | 2 +- openhands/core/config/openhands_config.py | 9 ++ openhands/resolver/send_pull_request.py | 46 +++++- openhands/runtime/action_execution_server.py | 30 +++- openhands/runtime/utils/command.py | 4 + .../resolver/github/test_send_pull_request.py | 149 +++++++++++++++++- .../gitlab/test_gitlab_send_pull_request.py | 33 +++- tests/unit/test_git_config.py | 76 +++++++++ 10 files changed, 334 insertions(+), 19 deletions(-) create mode 100644 tests/unit/test_git_config.py diff --git a/openhands/agenthub/codeact_agent/prompts/system_prompt.j2 b/openhands/agenthub/codeact_agent/prompts/system_prompt.j2 index de1cfc6569..17d7fad22d 100644 --- a/openhands/agenthub/codeact_agent/prompts/system_prompt.j2 +++ b/openhands/agenthub/codeact_agent/prompts/system_prompt.j2 @@ -31,7 +31,7 @@ Your primary role is to assist users by executing commands, modifying code, and -* When committing changes, you MUST use the `--author` flag to set the author to `"openhands "`. For example: `git commit --author="openhands " -m "Fix bug"`. This ensures all commits are attributed to the OpenHands agent, regardless of the local git config. +* If there are existing git user credentials already configured, use them and add Co-authored-by: openhands to any commits messages you make. if a git config doesn't exist use "openhands" as the user.name and "openhands@all-hands.dev" as the user.email by default, unless explicitly instructed otherwise. * Exercise caution with git operations. Do NOT make potentially dangerous changes (e.g., pushing to main, deleting repositories) unless explicitly asked to do so. * When committing changes, use `git status` to see all modified files, and stage all files necessary for the commit. Use `git commit -a` whenever possible. * Do NOT commit files that typically shouldn't go into version control (e.g., node_modules/, .env files, build directories, cache files, large binaries) unless explicitly instructed by the user. diff --git a/openhands/agenthub/codeact_agent/prompts/system_prompt_interactive.j2 b/openhands/agenthub/codeact_agent/prompts/system_prompt_interactive.j2 index 14f9a6f248..f5483a8f65 100644 --- a/openhands/agenthub/codeact_agent/prompts/system_prompt_interactive.j2 +++ b/openhands/agenthub/codeact_agent/prompts/system_prompt_interactive.j2 @@ -25,7 +25,7 @@ Your primary role is to assist users by executing commands, modifying code, and -* When committing changes, you MUST use the `--author` flag to set the author to `"openhands "`. For example: `git commit --author="openhands " -m "Fix bug"`. This ensures all commits are attributed to the OpenHands agent, regardless of the local git config. +* If there are existing git user credentials already configured, use them and add Co-authored-by: openhands to any commits messages you make. if a git config doesn't exist use "openhands" as the user.name and "openhands@all-hands.dev" as the user.email by default, unless explicitly instructed otherwise. * Exercise caution with git operations. Do NOT make potentially dangerous changes (e.g., pushing to main, deleting repositories) unless explicitly asked to do so. * When committing changes, use `git status` to see all modified files, and stage all files necessary for the commit. Use `git commit -a` whenever possible. * Do NOT commit files that typically shouldn't go into version control (e.g., node_modules/, .env files, build directories, cache files, large binaries) unless explicitly instructed by the user. diff --git a/openhands/agenthub/codeact_agent/prompts/system_prompt_long_horizon.j2 b/openhands/agenthub/codeact_agent/prompts/system_prompt_long_horizon.j2 index 39c06c4867..567a94cfa2 100644 --- a/openhands/agenthub/codeact_agent/prompts/system_prompt_long_horizon.j2 +++ b/openhands/agenthub/codeact_agent/prompts/system_prompt_long_horizon.j2 @@ -25,7 +25,7 @@ Your primary role is to assist users by executing commands, modifying code, and -* When committing changes, you MUST use the `--author` flag to set the author to `"openhands "`. For example: `git commit --author="openhands " -m "Fix bug"`. This ensures all commits are attributed to the OpenHands agent, regardless of the local git config. +* If there are existing git user credentials already configured, use them and add Co-authored-by: openhands to any commits messages you make. if a git config doesn't exist use "openhands" as the user.name and "openhands@all-hands.dev" as the user.email by default, unless explicitly instructed otherwise. * Exercise caution with git operations. Do NOT make potentially dangerous changes (e.g., pushing to main, deleting repositories) unless explicitly asked to do so. * When committing changes, use `git status` to see all modified files, and stage all files necessary for the commit. Use `git commit -a` whenever possible. * Do NOT commit files that typically shouldn't go into version control (e.g., node_modules/, .env files, build directories, cache files, large binaries) unless explicitly instructed by the user. diff --git a/openhands/core/config/openhands_config.py b/openhands/core/config/openhands_config.py index 790c990d96..65ea78ddce 100644 --- a/openhands/core/config/openhands_config.py +++ b/openhands/core/config/openhands_config.py @@ -57,6 +57,8 @@ class OpenHandsConfig(BaseModel): input is read line by line. When enabled, input continues until /exit command. mcp_host: Host for OpenHands' default MCP server mcp: MCP configuration settings. + git_user_name: Git user name for commits made by the agent. + git_user_email: Git user email for commits made by the agent. """ llms: dict[str, LLMConfig] = Field(default_factory=dict) @@ -112,6 +114,13 @@ class OpenHandsConfig(BaseModel): mcp: MCPConfig = Field(default_factory=MCPConfig) kubernetes: KubernetesConfig = Field(default_factory=KubernetesConfig) cli: CLIConfig = Field(default_factory=CLIConfig) + git_user_name: str = Field( + default='openhands', description='Git user name for commits made by the agent' + ) + git_user_email: str = Field( + default='openhands@all-hands.dev', + description='Git user email for commits made by the agent', + ) defaults_dict: ClassVar[dict] = {} diff --git a/openhands/resolver/send_pull_request.py b/openhands/resolver/send_pull_request.py index b8196f8c80..71a3b9fca1 100644 --- a/openhands/resolver/send_pull_request.py +++ b/openhands/resolver/send_pull_request.py @@ -159,13 +159,21 @@ def initialize_repo( return dest_dir -def make_commit(repo_dir: str, issue: Issue, issue_type: str) -> None: +def make_commit( + repo_dir: str, + issue: Issue, + issue_type: str, + git_user_name: str = 'openhands', + git_user_email: str = 'openhands@all-hands.dev', +) -> None: """Make a commit with the changes to the repository. Args: repo_dir: The directory containing the repository issue: The issue to fix issue_type: The type of the issue + git_user_name: Git username for commits + git_user_email: Git email for commits """ # Check if git username is set result = subprocess.run( @@ -176,15 +184,15 @@ def make_commit(repo_dir: str, issue: Issue, issue_type: str) -> None: ) if not result.stdout.strip(): - # If username is not set, configure git + # If username is not set, configure git with the provided credentials subprocess.run( - f'git -C {repo_dir} config user.name "openhands" && ' - f'git -C {repo_dir} config user.email "openhands@all-hands.dev" && ' + f'git -C {repo_dir} config user.name "{git_user_name}" && ' + f'git -C {repo_dir} config user.email "{git_user_email}" && ' f'git -C {repo_dir} config alias.git "git --no-pager"', shell=True, check=True, ) - logger.info('Git user configured as openhands') + logger.info(f'Git user configured as {git_user_name} <{git_user_email}>') # Add all changes to the git index result = subprocess.run( @@ -235,6 +243,8 @@ def send_pull_request( reviewer: str | None = None, pr_title: str | None = None, base_domain: str | None = None, + git_user_name: str = 'openhands', + git_user_email: str = 'openhands@all-hands.dev', ) -> str: """Send a pull request to a GitHub, GitLab, or Bitbucket repository. @@ -503,6 +513,8 @@ def process_single_issue( reviewer: str | None = None, pr_title: str | None = None, base_domain: str | None = None, + git_user_name: str = 'openhands', + git_user_email: str = 'openhands@all-hands.dev', ) -> None: # Determine default base_domain based on platform if base_domain is None: @@ -534,7 +546,13 @@ def process_single_issue( apply_patch(patched_repo_dir, resolver_output.git_patch) - make_commit(patched_repo_dir, resolver_output.issue, issue_type) + make_commit( + patched_repo_dir, + resolver_output.issue, + issue_type, + git_user_name, + git_user_email, + ) if issue_type == 'pr': update_existing_pull_request( @@ -561,6 +579,8 @@ def process_single_issue( reviewer=reviewer, pr_title=pr_title, base_domain=base_domain, + git_user_name=git_user_name, + git_user_email=git_user_email, ) @@ -658,6 +678,18 @@ def main() -> None: default=None, help='Base domain for the git server (defaults to "github.com" for GitHub and "gitlab.com" for GitLab)', ) + parser.add_argument( + '--git-user-name', + type=str, + default='openhands', + help='Git user name for commits', + ) + parser.add_argument( + '--git-user-email', + type=str, + default='openhands@all-hands.dev', + help='Git user email for commits', + ) my_args = parser.parse_args() token = my_args.token or os.getenv('GITHUB_TOKEN') or os.getenv('GITLAB_TOKEN') @@ -705,6 +737,8 @@ def main() -> None: my_args.reviewer, my_args.pr_title, my_args.base_domain, + my_args.git_user_name, + my_args.git_user_email, ) diff --git a/openhands/runtime/action_execution_server.py b/openhands/runtime/action_execution_server.py index caab652919..ac45a48086 100644 --- a/openhands/runtime/action_execution_server.py +++ b/openhands/runtime/action_execution_server.py @@ -177,11 +177,15 @@ 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 ) @@ -350,10 +354,10 @@ class ActionExecutor: if is_windows: # Windows, local - split into separate commands INIT_COMMANDS.append( - 'git config --file ./.git_config user.name "openhands"' + f'git config --file ./.git_config user.name "{self.git_user_name}"' ) INIT_COMMANDS.append( - 'git config --file ./.git_config user.email "openhands@all-hands.dev"' + f'git config --file ./.git_config user.email "{self.git_user_email}"' ) INIT_COMMANDS.append( '$env:GIT_CONFIG = (Join-Path (Get-Location) ".git_config")' @@ -361,16 +365,16 @@ class ActionExecutor: else: # Linux/macOS, local base_git_config = ( - 'git config --file ./.git_config user.name "openhands" && ' - 'git config --file ./.git_config user.email "openhands@all-hands.dev" && ' + f'git config --file ./.git_config user.name "{self.git_user_name}" && ' + f'git config --file ./.git_config user.email "{self.git_user_email}" && ' 'export GIT_CONFIG=$(pwd)/.git_config' ) INIT_COMMANDS.append(base_git_config) else: # Non-local (implies Linux/macOS) base_git_config = ( - 'git config --global user.name "openhands" && ' - 'git config --global user.email "openhands@all-hands.dev"' + f'git config --global user.name "{self.git_user_name}" && ' + f'git config --global user.email "{self.git_user_email}"' ) INIT_COMMANDS.append(base_git_config) @@ -692,6 +696,18 @@ 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() @@ -725,6 +741,8 @@ 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.') diff --git a/openhands/runtime/utils/command.py b/openhands/runtime/utils/command.py index 18a3d85117..4c9c6342b7 100644 --- a/openhands/runtime/utils/command.py +++ b/openhands/runtime/utils/command.py @@ -57,6 +57,10 @@ 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, ] diff --git a/tests/unit/resolver/github/test_send_pull_request.py b/tests/unit/resolver/github/test_send_pull_request.py index ba1a359cba..3008d3c57f 100644 --- a/tests/unit/resolver/github/test_send_pull_request.py +++ b/tests/unit/resolver/github/test_send_pull_request.py @@ -874,7 +874,11 @@ def test_process_single_pr_update( f'{mock_output_dir}/patches/pr_1', resolver_output.git_patch ) mock_make_commit.assert_called_once_with( - f'{mock_output_dir}/patches/pr_1', resolver_output.issue, 'pr' + f'{mock_output_dir}/patches/pr_1', + resolver_output.issue, + 'pr', + 'openhands', + 'openhands@all-hands.dev', ) mock_update_existing_pull_request.assert_called_once_with( issue=resolver_output.issue, @@ -952,7 +956,11 @@ def test_process_single_issue( f'{mock_output_dir}/patches/issue_1', resolver_output.git_patch ) mock_make_commit.assert_called_once_with( - f'{mock_output_dir}/patches/issue_1', resolver_output.issue, 'issue' + f'{mock_output_dir}/patches/issue_1', + resolver_output.issue, + 'issue', + 'openhands', + 'openhands@all-hands.dev', ) mock_send_pull_request.assert_called_once_with( issue=resolver_output.issue, @@ -967,6 +975,8 @@ def test_process_single_issue( reviewer=None, pr_title=None, base_domain='github.com', + git_user_name='openhands', + git_user_email='openhands@all-hands.dev', ) @@ -1163,6 +1173,8 @@ def test_main( mock_args.reviewer, mock_args.pr_title, ANY, + ANY, # git_user_name from args + ANY, # git_user_email from args ) # Other assertions @@ -1255,6 +1267,139 @@ def test_make_commit_no_changes(mock_subprocess_run): assert f'git -C {repo_dir} status --porcelain' in git_status_call +@patch('subprocess.run') +def test_make_commit_with_custom_git_config(mock_subprocess_run): + # Setup + repo_dir = '/path/to/repo' + issue = Issue( + owner='test-owner', + repo='test-repo', + number=42, + title='Test Issue', + body='Test body', + ) + custom_git_user_name = 'custom-user' + custom_git_user_email = 'custom@example.com' + + # Mock subprocess.run to simulate successful operations + mock_subprocess_run.side_effect = [ + MagicMock( + returncode=0, stdout='' + ), # git config user.name check (empty = not set) + MagicMock(returncode=0), # git config set user.name and user.email + MagicMock(returncode=0), # git add + MagicMock(returncode=0, stdout='modified files'), # git status --porcelain + MagicMock(returncode=0), # git commit + ] + + # Call the function with custom git config + make_commit(repo_dir, issue, 'issue', custom_git_user_name, custom_git_user_email) + + # Assert that subprocess.run was called with the correct arguments + calls = mock_subprocess_run.call_args_list + assert len(calls) == 5 + + # Check git config check call (first call) + git_config_check_call = calls[0][0][0] + assert git_config_check_call == f'git -C {repo_dir} config user.name' + + # Check git config set call (second call) + git_config_set_call = calls[1][0][0] + expected_config_command = ( + f'git -C {repo_dir} config user.name "{custom_git_user_name}" && ' + f'git -C {repo_dir} config user.email "{custom_git_user_email}" && ' + f'git -C {repo_dir} config alias.git "git --no-pager"' + ) + assert expected_config_command == git_config_set_call + + +@patch('subprocess.run') +def test_make_commit_with_existing_git_config(mock_subprocess_run): + # Setup + repo_dir = '/path/to/repo' + issue = Issue( + owner='test-owner', + repo='test-repo', + number=42, + title='Test Issue', + body='Test body', + ) + + # Mock subprocess.run to simulate successful operations + mock_subprocess_run.side_effect = [ + MagicMock( + returncode=0, stdout='existing-user\n' + ), # git config user.name check (non-empty = already set) + MagicMock(returncode=0), # git add + MagicMock(returncode=0, stdout='modified files'), # git status --porcelain + MagicMock(returncode=0), # git commit + ] + + # Call the function + make_commit(repo_dir, issue, 'issue') + + # Assert that subprocess.run was called with the correct arguments + calls = mock_subprocess_run.call_args_list + assert len(calls) == 4 + + # Check git config check call (first call) + git_config_check_call = calls[0][0][0] + assert git_config_check_call == f'git -C {repo_dir} config user.name' + + # Check that git config set was NOT called (since username already exists) + # The remaining calls should be git add, git status, git commit + git_add_call = calls[1][0][0] + assert f'git -C {repo_dir} add .' == git_add_call + + +@patch('subprocess.run') +def test_make_commit_with_special_characters_in_git_config(mock_subprocess_run): + # Setup + repo_dir = '/path/to/repo' + issue = Issue( + owner='test-owner', + repo='test-repo', + number=42, + title='Test Issue', + body='Test body', + ) + git_user_name = 'User "with quotes"' + git_user_email = 'user@domain.com' + + # Mock subprocess.run to simulate successful operations + mock_subprocess_run.side_effect = [ + MagicMock( + returncode=0, stdout='' + ), # git config user.name check (empty = not set) + MagicMock(returncode=0), # git config set + MagicMock(returncode=0), # git add + MagicMock(returncode=0, stdout='modified files'), # git status --porcelain + MagicMock(returncode=0), # git commit + ] + + # Call the function with special characters in git config + make_commit(repo_dir, issue, 'issue', git_user_name, git_user_email) + + # Assert that subprocess.run was called with properly escaped git config + calls = mock_subprocess_run.call_args_list + assert len(calls) == 5 + + # Check git config check call (first call) + git_config_check_call = calls[0][0][0] + assert git_config_check_call == f'git -C {repo_dir} config user.name' + + # Check git config set call (second call) + git_config_set_call = calls[1][0][0] + + # Check that quotes are properly handled in the command + expected_config_command = ( + f'git -C {repo_dir} config user.name "{git_user_name}" && ' + f'git -C {repo_dir} config user.email "{git_user_email}" && ' + f'git -C {repo_dir} config alias.git "git --no-pager"' + ) + assert expected_config_command == git_config_set_call + + def test_apply_patch_rename_directory(mock_output_dir): # Create a sample directory structure old_dir = os.path.join(mock_output_dir, 'prompts', 'resolve') diff --git a/tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py b/tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py index c920d4a096..d342273fbe 100644 --- a/tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py +++ b/tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py @@ -768,6 +768,11 @@ def test_process_single_pr_update( None, False, None, + None, + None, + None, + 'openhands', + 'openhands@all-hands.dev', ) mock_initialize_repo.assert_called_once_with(mock_output_dir, 1, 'pr', 'branch 1') @@ -775,7 +780,11 @@ def test_process_single_pr_update( f'{mock_output_dir}/patches/pr_1', resolver_output.git_patch ) mock_make_commit.assert_called_once_with( - f'{mock_output_dir}/patches/pr_1', resolver_output.issue, 'pr' + f'{mock_output_dir}/patches/pr_1', + resolver_output.issue, + 'pr', + 'openhands', + 'openhands@all-hands.dev', ) mock_update_existing_pull_request.assert_called_once_with( issue=resolver_output.issue, @@ -845,6 +854,11 @@ def test_process_single_issue( None, False, None, + None, + None, + None, + 'openhands', + 'openhands@all-hands.dev', ) # Assert that the mocked functions were called with correct arguments @@ -853,7 +867,11 @@ def test_process_single_issue( f'{mock_output_dir}/patches/issue_1', resolver_output.git_patch ) mock_make_commit.assert_called_once_with( - f'{mock_output_dir}/patches/issue_1', resolver_output.issue, 'issue' + f'{mock_output_dir}/patches/issue_1', + resolver_output.issue, + 'issue', + 'openhands', + 'openhands@all-hands.dev', ) mock_send_pull_request.assert_called_once_with( issue=resolver_output.issue, @@ -868,6 +886,8 @@ def test_process_single_issue( reviewer=None, pr_title=None, base_domain='gitlab.com', + git_user_name='openhands', + git_user_email='openhands@all-hands.dev', ) @@ -920,6 +940,11 @@ def test_process_single_issue_unsuccessful( None, False, None, + None, + None, + None, + 'openhands', + 'openhands@all-hands.dev', ) # Assert that none of the mocked functions were called @@ -1023,6 +1048,8 @@ def test_main( mock_args.reviewer = None mock_args.pr_title = None mock_args.selected_repo = None + mock_args.git_user_name = 'openhands' + mock_args.git_user_email = 'openhands@all-hands.dev' mock_parser.return_value.parse_args.return_value = mock_args # Setup environment variables @@ -1065,6 +1092,8 @@ def test_main( mock_args.reviewer, mock_args.pr_title, ANY, + mock_args.git_user_name, + mock_args.git_user_email, ) # Other assertions diff --git a/tests/unit/test_git_config.py b/tests/unit/test_git_config.py new file mode 100644 index 0000000000..1603ac8109 --- /dev/null +++ b/tests/unit/test_git_config.py @@ -0,0 +1,76 @@ +"""Tests for git configuration functionality.""" + +import os +from unittest.mock import patch + +from openhands.core.config import OpenHandsConfig, load_from_env +from openhands.runtime.utils.command import get_action_execution_server_startup_command + + +class TestGitConfig: + """Test git configuration functionality.""" + + def test_default_git_config(self): + """Test that default git configuration is set correctly.""" + config = OpenHandsConfig() + assert config.git_user_name == 'openhands' + assert config.git_user_email == 'openhands@all-hands.dev' + + def test_git_config_from_env_vars(self): + """Test that git configuration can be set via environment variables.""" + with patch.dict( + os.environ, + {'GIT_USER_NAME': 'testuser', 'GIT_USER_EMAIL': 'testuser@example.com'}, + ): + config = OpenHandsConfig() + load_from_env(config, os.environ) + + 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.""" + config = OpenHandsConfig() + config.git_user_name = 'customuser' + config.git_user_email = 'customuser@example.com' + + cmd = get_action_execution_server_startup_command( + server_port=8000, + plugins=[], + app_config=config, + python_prefix=['python'], + 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 + + def test_git_config_with_special_characters(self): + """Test that git configuration handles special characters correctly.""" + config = OpenHandsConfig() + config.git_user_name = 'User With Spaces' + config.git_user_email = 'user+tag@example.com' + + cmd = get_action_execution_server_startup_command( + server_port=8000, + plugins=[], + app_config=config, + python_prefix=['python'], + python_executable='python', + ) + + assert 'User With Spaces' in cmd + assert 'user+tag@example.com' in cmd + + def test_git_config_empty_values(self): + """Test behavior with empty git configuration values.""" + with patch.dict(os.environ, {'GIT_USER_NAME': '', 'GIT_USER_EMAIL': ''}): + config = OpenHandsConfig() + load_from_env(config, os.environ) + + # Empty values should fall back to defaults + assert config.git_user_name == 'openhands' + assert config.git_user_email == 'openhands@all-hands.dev'