diff --git a/aider/args.py b/aider/args.py index 6df19778b..dfa84b6c2 100644 --- a/aider/args.py +++ b/aider/args.py @@ -427,14 +427,20 @@ def get_parser(default_config_files, git_root): group.add_argument( "--attribute-author", action=argparse.BooleanOptionalAction, - default=True, - help="Attribute aider code changes in the git author name (default: True)", + default=None, + help=( + "Attribute aider code changes in the git author name (default: True). If explicitly set" + " to True, overrides --attribute-co-authored-by precedence." + ), ) group.add_argument( "--attribute-committer", action=argparse.BooleanOptionalAction, - default=True, - help="Attribute aider commits in the git committer name (default: True)", + default=None, + help=( + "Attribute aider commits in the git committer name (default: True). If explicitly set" + " to True, overrides --attribute-co-authored-by precedence for aider edits." + ), ) group.add_argument( "--attribute-commit-message-author", @@ -448,6 +454,16 @@ def get_parser(default_config_files, git_root): default=False, help="Prefix all commit messages with 'aider: ' (default: False)", ) + group.add_argument( + "--attribute-co-authored-by", + action=argparse.BooleanOptionalAction, + default=False, + help=( + "Attribute aider edits using the Co-authored-by trailer in the commit message" + " (default: False). If True, this takes precedence over default --attribute-author and" + " --attribute-committer behavior unless they are explicitly set to True." + ), + ) group.add_argument( "--git-commit-verify", action=argparse.BooleanOptionalAction, diff --git a/aider/coders/base_coder.py b/aider/coders/base_coder.py index bf4eb635d..b36ad5098 100755 --- a/aider/coders/base_coder.py +++ b/aider/coders/base_coder.py @@ -2331,7 +2331,7 @@ class Coder: context = self.get_context_from_history(self.cur_messages) try: - res = self.repo.commit(fnames=edited, context=context, aider_edits=True) + res = self.repo.commit(fnames=edited, context=context, aider_edits=True, coder=self) if res: self.show_auto_commit_outcome(res) commit_hash, commit_message = res @@ -2367,7 +2367,7 @@ class Coder: if not self.repo: return - self.repo.commit(fnames=self.need_commit_before_edits) + self.repo.commit(fnames=self.need_commit_before_edits, coder=self) # files changed, move cur messages back behind the files messages # self.move_back_cur_messages(self.gpt_prompts.files_content_local_edits) diff --git a/aider/main.py b/aider/main.py index 89286e1de..a37f16129 100644 --- a/aider/main.py +++ b/aider/main.py @@ -904,6 +904,7 @@ def main(argv=None, input=None, output=None, force_git_root=None, return_coder=F commit_prompt=args.commit_prompt, subtree_only=args.subtree_only, git_commit_verify=args.git_commit_verify, + attribute_co_authored_by=args.attribute_co_authored_by, # Pass the arg ) except FileNotFoundError: pass diff --git a/aider/repo.py b/aider/repo.py index 5ece5147c..aa2d525f7 100644 --- a/aider/repo.py +++ b/aider/repo.py @@ -1,3 +1,4 @@ +import contextlib import os import time from pathlib import Path, PurePosixPath @@ -34,6 +35,19 @@ ANY_GIT_ERROR += [ ANY_GIT_ERROR = tuple(ANY_GIT_ERROR) +@contextlib.contextmanager +def set_git_env(var_name, value, original_value): + """Temporarily set a Git environment variable.""" + os.environ[var_name] = value + try: + yield + finally: + if original_value is not None: + os.environ[var_name] = original_value + elif var_name in os.environ: + del os.environ[var_name] + + class GitRepo: repo = None aider_ignore_file = None @@ -58,6 +72,7 @@ class GitRepo: commit_prompt=None, subtree_only=False, git_commit_verify=True, + attribute_co_authored_by=False, # Added parameter ): self.io = io self.models = models @@ -69,6 +84,7 @@ class GitRepo: self.attribute_committer = attribute_committer self.attribute_commit_message_author = attribute_commit_message_author self.attribute_commit_message_committer = attribute_commit_message_committer + self.attribute_co_authored_by = attribute_co_authored_by # Assign from parameter self.commit_prompt = commit_prompt self.subtree_only = subtree_only self.git_commit_verify = git_commit_verify @@ -111,7 +127,71 @@ class GitRepo: if aider_ignore_file: self.aider_ignore_file = Path(aider_ignore_file) - def commit(self, fnames=None, context=None, message=None, aider_edits=False): + def commit(self, fnames=None, context=None, message=None, aider_edits=False, coder=None): + """ + Commit the specified files or all dirty files if none are specified. + + Args: + fnames (list, optional): List of filenames to commit. Defaults to None (commit all + dirty files). + context (str, optional): Context for generating the commit message. Defaults to None. + message (str, optional): Explicit commit message. Defaults to None (generate message). + aider_edits (bool, optional): Whether the changes were made by Aider. Defaults to False. + This affects attribution logic. + coder (Coder, optional): The Coder instance, used to access config and model info. + Defaults to None. + + Returns: + tuple(str, str) or None: The commit hash and commit message if successful, else None. + + Attribution Logic: + ------------------ + This method handles Git commit attribution based on configuration flags and whether + Aider generated the changes (`aider_edits`). + + Key Concepts: + - Author: The person who originally wrote the code changes. + - Committer: The person who last applied the commit to the repository. + - aider_edits=True: Changes were generated by Aider (LLM). + - aider_edits=False: Commit is user-driven (e.g., /commit manually staged changes). + - Explicit Setting: A flag (--attribute-...) is set to True or False via command line + or config file. + - Implicit Default: A flag is not explicitly set, defaulting to None in args, which is + interpreted as True unless overridden by other logic. + + Flags: + - --attribute-author: Modify Author name to "User Name (aider)". + - --attribute-committer: Modify Committer name to "User Name (aider)". + - --attribute-co-authored-by: Add "Co-authored-by: aider () " + trailer to the commit message. + + Behavior Summary: + + 1. When aider_edits = True (AI Changes): + - If --attribute-co-authored-by=True: + - Co-authored-by trailer IS ADDED. + - Author/Committer names are NOT modified by default (co-authored-by takes precedence). + - EXCEPTION: If --attribute-author/--attribute-committer is EXPLICITLY True, + the respective name IS modified (explicit overrides precedence). + - If --attribute-co-authored-by=False: + - Co-authored-by trailer is NOT added. + - Author/Committer names ARE modified by default (implicit True). + - EXCEPTION: If --attribute-author/--attribute-committer is EXPLICITLY False, + the respective name is NOT modified. + + 2. When aider_edits = False (User Changes): + - --attribute-co-authored-by is IGNORED (trailer never added). + - Author name is NEVER modified (--attribute-author ignored). + - Committer name IS modified by default (implicit True, as Aider runs `git commit`). + - EXCEPTION: If --attribute-committer is EXPLICITLY False, the name is NOT modified. + + Resulting Scenarios: + - Standard AI edit (defaults): Co-authored-by=False -> Author=You(aider), Committer=You(aider) + - AI edit with Co-authored-by (default): Co-authored-by=True -> Author=You, Committer=You, Trailer added + - AI edit with Co-authored-by + Explicit Author: Co-authored-by=True, --attribute-author -> Author=You(aider), Committer=You, Trailer added + - User commit (defaults): aider_edits=False -> Author=You, Committer=You(aider) + - User commit with explicit no-committer: aider_edits=False, --no-attribute-committer -> Author=You, Committer=You + """ if not fnames and not self.repo.is_dirty(): return @@ -124,17 +204,68 @@ class GitRepo: else: commit_message = self.get_commit_message(diffs, context) - if aider_edits and self.attribute_commit_message_author: - commit_message = "aider: " + commit_message - elif self.attribute_commit_message_committer: - commit_message = "aider: " + commit_message + # Retrieve attribute settings, prioritizing coder.args if available + if coder and hasattr(coder, "args"): + attribute_author = coder.args.attribute_author + attribute_committer = coder.args.attribute_committer + attribute_commit_message_author = coder.args.attribute_commit_message_author + attribute_commit_message_committer = coder.args.attribute_commit_message_committer + attribute_co_authored_by = coder.args.attribute_co_authored_by + else: + # Fallback to self attributes (initialized from config/defaults) + attribute_author = self.attribute_author + attribute_committer = self.attribute_committer + attribute_commit_message_author = self.attribute_commit_message_author + attribute_commit_message_committer = self.attribute_commit_message_committer + attribute_co_authored_by = self.attribute_co_authored_by + + # Determine explicit settings (None means use default behavior) + author_explicit = attribute_author is not None + committer_explicit = attribute_committer is not None + + # Determine effective settings (apply default True if not explicit) + effective_author = True if attribute_author is None else attribute_author + effective_committer = True if attribute_committer is None else attribute_committer + + + # Determine commit message prefixing + prefix_commit_message = aider_edits and ( + attribute_commit_message_author or attribute_commit_message_committer + ) + + # Determine Co-authored-by trailer + commit_message_trailer = "" + if aider_edits and attribute_co_authored_by: + model_name = "unknown-model" + if coder and hasattr(coder, "main_model") and coder.main_model.name: + model_name = coder.main_model.name + commit_message_trailer = ( + f"\n\nCo-authored-by: aider ({model_name}) " + ) + + # Determine if author/committer names should be modified + # Author modification applies only to aider edits. + # It's used if effective_author is True AND (co-authored-by is False OR author was explicitly set). + use_attribute_author = ( + aider_edits + and effective_author + and (not attribute_co_authored_by or author_explicit) + ) + + # Committer modification applies regardless of aider_edits (based on tests). + # It's used if effective_committer is True AND (it's not an aider edit with co-authored-by OR committer was explicitly set). + use_attribute_committer = effective_committer and ( + not (aider_edits and attribute_co_authored_by) or committer_explicit + ) + if not commit_message: commit_message = "(no commit message provided)" - full_commit_message = commit_message - # if context: - # full_commit_message += "\n\n# Aider chat conversation:\n\n" + context + if prefix_commit_message: + commit_message = "aider: " + commit_message + + full_commit_message = commit_message + commit_message_trailer cmd = ["-m", full_commit_message] if not self.git_commit_verify: @@ -152,36 +283,30 @@ class GitRepo: original_user_name = self.repo.git.config("--get", "user.name") original_committer_name_env = os.environ.get("GIT_COMMITTER_NAME") + original_author_name_env = os.environ.get("GIT_AUTHOR_NAME") committer_name = f"{original_user_name} (aider)" - if self.attribute_committer: - os.environ["GIT_COMMITTER_NAME"] = committer_name - - if aider_edits and self.attribute_author: - original_author_name_env = os.environ.get("GIT_AUTHOR_NAME") - os.environ["GIT_AUTHOR_NAME"] = committer_name - try: - self.repo.git.commit(cmd) - commit_hash = self.get_head_commit_sha(short=True) - self.io.tool_output(f"Commit {commit_hash} {commit_message}", bold=True) - return commit_hash, commit_message + # Use context managers to handle environment variables + with contextlib.ExitStack() as stack: + if use_attribute_committer: + stack.enter_context( + set_git_env("GIT_COMMITTER_NAME", committer_name, original_committer_name_env) + ) + if use_attribute_author: + stack.enter_context( + set_git_env("GIT_AUTHOR_NAME", committer_name, original_author_name_env) + ) + + # Perform the commit + self.repo.git.commit(cmd) + commit_hash = self.get_head_commit_sha(short=True) + self.io.tool_output(f"Commit {commit_hash} {commit_message}", bold=True) + return commit_hash, commit_message + except ANY_GIT_ERROR as err: self.io.tool_error(f"Unable to commit: {err}") - finally: - # Restore the env - - if self.attribute_committer: - if original_committer_name_env is not None: - os.environ["GIT_COMMITTER_NAME"] = original_committer_name_env - else: - del os.environ["GIT_COMMITTER_NAME"] - - if aider_edits and self.attribute_author: - if original_author_name_env is not None: - os.environ["GIT_AUTHOR_NAME"] = original_author_name_env - else: - del os.environ["GIT_AUTHOR_NAME"] + # No return here, implicitly returns None def get_rel_repo_dir(self): try: diff --git a/tests/basic/test_repo.py b/tests/basic/test_repo.py index aa863c570..400c307a8 100644 --- a/tests/basic/test_repo.py +++ b/tests/basic/test_repo.py @@ -4,7 +4,7 @@ import tempfile import time import unittest from pathlib import Path -from unittest.mock import patch +from unittest.mock import MagicMock, patch import git @@ -165,14 +165,11 @@ class TestRepo(unittest.TestCase): args = mock_send.call_args[0] # Get positional args self.assertEqual(args[0][0]["content"], custom_prompt) # Check first message content + @unittest.skipIf(platform.system() == "Windows", "Git env var behavior differs on Windows") @patch("aider.repo.GitRepo.get_commit_message") def test_commit_with_custom_committer_name(self, mock_send): mock_send.return_value = '"a good commit message"' - # Cleanup of the git temp dir explodes on windows - if platform.system() == "Windows": - return - with GitTemporaryDirectory(): # new repo raw_repo = git.Repo() @@ -185,32 +182,192 @@ class TestRepo(unittest.TestCase): raw_repo.git.commit("-m", "initial commit") io = InputOutput() - git_repo = GitRepo(io, None, None) + # Initialize GitRepo with default None values for attributes + git_repo = GitRepo(io, None, None, attribute_author=None, attribute_committer=None) - # commit a change + # commit a change with aider_edits=True (using default attributes) fname.write_text("new content") - git_repo.commit(fnames=[str(fname)], aider_edits=True) + commit_result = git_repo.commit(fnames=[str(fname)], aider_edits=True) + self.assertIsNotNone(commit_result) - # check the committer name + # check the committer name (defaults interpreted as True) commit = raw_repo.head.commit self.assertEqual(commit.author.name, "Test User (aider)") self.assertEqual(commit.committer.name, "Test User (aider)") - # commit a change without aider_edits + # commit a change without aider_edits (using default attributes) fname.write_text("new content again!") - git_repo.commit(fnames=[str(fname)], aider_edits=False) + commit_result = git_repo.commit(fnames=[str(fname)], aider_edits=False) + self.assertIsNotNone(commit_result) - # check the committer name + # check the committer name (author not modified, committer still modified by default) commit = raw_repo.head.commit self.assertEqual(commit.author.name, "Test User") self.assertEqual(commit.committer.name, "Test User (aider)") + # Now test with explicit False + git_repo_explicit_false = GitRepo(io, None, None, attribute_author=False, attribute_committer=False) + fname.write_text("explicit false content") + commit_result = git_repo_explicit_false.commit(fnames=[str(fname)], aider_edits=True) + self.assertIsNotNone(commit_result) + commit = raw_repo.head.commit + self.assertEqual(commit.author.name, "Test User") # Explicit False + self.assertEqual(commit.committer.name, "Test User") # Explicit False + # check that the original committer name is restored original_committer_name = os.environ.get("GIT_COMMITTER_NAME") self.assertIsNone(original_committer_name) original_author_name = os.environ.get("GIT_AUTHOR_NAME") self.assertIsNone(original_author_name) + # Test user commit with explicit no-committer attribution + git_repo_user_no_committer = GitRepo(io, None, None, attribute_committer=False) + fname.write_text("user no committer content") + commit_result = git_repo_user_no_committer.commit(fnames=[str(fname)], aider_edits=False) + self.assertIsNotNone(commit_result) + commit = raw_repo.head.commit + self.assertEqual(commit.author.name, "Test User", msg="Author name should not be modified for user commits") + self.assertEqual(commit.committer.name, "Test User", msg="Committer name should not be modified when attribute_committer=False") + + @unittest.skipIf(platform.system() == "Windows", "Git env var behavior differs on Windows") + def test_commit_with_co_authored_by(self): + with GitTemporaryDirectory(): + # new repo + raw_repo = git.Repo() + raw_repo.config_writer().set_value("user", "name", "Test User").release() + raw_repo.config_writer().set_value("user", "email", "test@example.com").release() + + # add a file and commit it + fname = Path("file.txt") + fname.touch() + raw_repo.git.add(str(fname)) + raw_repo.git.commit("-m", "initial commit") + + # Mock coder args: Co-authored-by enabled, author/committer use default (None) + mock_coder = MagicMock() + mock_coder.args.attribute_co_authored_by = True + mock_coder.args.attribute_author = None # Default + mock_coder.args.attribute_committer = None # Default + mock_coder.args.attribute_commit_message_author = False + mock_coder.args.attribute_commit_message_committer = False + # The code uses coder.main_model.name for the co-authored-by line + mock_coder.main_model = MagicMock() + mock_coder.main_model.name = "gpt-test" + + + io = InputOutput() + git_repo = GitRepo(io, None, None) + + # commit a change with aider_edits=True and co-authored-by flag + fname.write_text("new content") + commit_result = git_repo.commit(fnames=[str(fname)], aider_edits=True, coder=mock_coder, message="Aider edit") + self.assertIsNotNone(commit_result) + + # check the commit message and author/committer + commit = raw_repo.head.commit + self.assertIn("Co-authored-by: aider (gpt-test) ", commit.message) + self.assertEqual(commit.message.splitlines()[0], "Aider edit") + # With default (None), co-authored-by takes precedence + self.assertEqual(commit.author.name, "Test User", msg="Author name should not be modified when co-authored-by takes precedence") + self.assertEqual(commit.committer.name, "Test User", msg="Committer name should not be modified when co-authored-by takes precedence") + + @unittest.skipIf(platform.system() == "Windows", "Git env var behavior differs on Windows") + def test_commit_co_authored_by_with_explicit_name_modification(self): + # Test scenario where Co-authored-by is true AND author/committer modification are explicitly True + with GitTemporaryDirectory(): + # Setup repo... + # new repo + raw_repo = git.Repo() + raw_repo.config_writer().set_value("user", "name", "Test User").release() + raw_repo.config_writer().set_value("user", "email", "test@example.com").release() + + # add a file and commit it + fname = Path("file.txt") + fname.touch() + raw_repo.git.add(str(fname)) + raw_repo.git.commit("-m", "initial commit") + + # Mock coder args: Co-authored-by enabled, author/committer modification explicitly enabled + mock_coder = MagicMock() + mock_coder.args.attribute_co_authored_by = True + mock_coder.args.attribute_author = True # Explicitly enable + mock_coder.args.attribute_committer = True # Explicitly enable + mock_coder.args.attribute_commit_message_author = False + mock_coder.args.attribute_commit_message_committer = False + mock_coder.main_model = MagicMock() + mock_coder.main_model.name = "gpt-test-combo" + + + io = InputOutput() + git_repo = GitRepo(io, None, None) + + # commit a change with aider_edits=True and combo flags + fname.write_text("new content combo") + commit_result = git_repo.commit(fnames=[str(fname)], aider_edits=True, coder=mock_coder, message="Aider combo edit") + self.assertIsNotNone(commit_result) + + # check the commit message and author/committer + commit = raw_repo.head.commit + self.assertIn("Co-authored-by: aider (gpt-test-combo) ", commit.message) + self.assertEqual(commit.message.splitlines()[0], "Aider combo edit") + # When co-authored-by is true BUT author/committer are explicit True, modification SHOULD happen + self.assertEqual(commit.author.name, "Test User (aider)", msg="Author name should be modified when explicitly True, even with co-author") + self.assertEqual(commit.committer.name, "Test User (aider)", msg="Committer name should be modified when explicitly True, even with co-author") + + @unittest.skipIf(platform.system() == "Windows", "Git env var behavior differs on Windows") + def test_commit_ai_edits_no_coauthor_explicit_false(self): + # Test AI edits (aider_edits=True) when co-authored-by is False, + # but author or committer attribution is explicitly disabled. + with GitTemporaryDirectory(): + # Setup repo + raw_repo = git.Repo() + raw_repo.config_writer().set_value("user", "name", "Test User").release() + raw_repo.config_writer().set_value("user", "email", "test@example.com").release() + fname = Path("file.txt") + fname.touch() + raw_repo.git.add(str(fname)) + raw_repo.git.commit("-m", "initial commit") + + io = InputOutput() + + # Case 1: attribute_author = False, attribute_committer = None (default True) + mock_coder_no_author = MagicMock() + mock_coder_no_author.args.attribute_co_authored_by = False + mock_coder_no_author.args.attribute_author = False # Explicit False + mock_coder_no_author.args.attribute_committer = None # Default True + mock_coder_no_author.args.attribute_commit_message_author = False + mock_coder_no_author.args.attribute_commit_message_committer = False + mock_coder_no_author.main_model = MagicMock() + mock_coder_no_author.main_model.name = "gpt-test-no-author" + + git_repo_no_author = GitRepo(io, None, None) + fname.write_text("no author content") + commit_result = git_repo_no_author.commit(fnames=[str(fname)], aider_edits=True, coder=mock_coder_no_author, message="Aider no author") + self.assertIsNotNone(commit_result) + commit = raw_repo.head.commit + self.assertNotIn("Co-authored-by:", commit.message) + self.assertEqual(commit.author.name, "Test User") # Explicit False + self.assertEqual(commit.committer.name, "Test User (aider)") # Default True + + # Case 2: attribute_author = None (default True), attribute_committer = False + mock_coder_no_committer = MagicMock() + mock_coder_no_committer.args.attribute_co_authored_by = False + mock_coder_no_committer.args.attribute_author = None # Default True + mock_coder_no_committer.args.attribute_committer = False # Explicit False + mock_coder_no_committer.args.attribute_commit_message_author = False + mock_coder_no_committer.args.attribute_commit_message_committer = False + mock_coder_no_committer.main_model = MagicMock() + mock_coder_no_committer.main_model.name = "gpt-test-no-committer" + + git_repo_no_committer = GitRepo(io, None, None) + fname.write_text("no committer content") + commit_result = git_repo_no_committer.commit(fnames=[str(fname)], aider_edits=True, coder=mock_coder_no_committer, message="Aider no committer") + self.assertIsNotNone(commit_result) + commit = raw_repo.head.commit + self.assertNotIn("Co-authored-by:", commit.message) + self.assertEqual(commit.author.name, "Test User (aider)", msg="Author name should be modified (default True) when co-author=False") + self.assertEqual(commit.committer.name, "Test User", msg="Committer name should not be modified (explicit False) when co-author=False") + def test_get_tracked_files(self): # Create a temporary directory tempdir = Path(tempfile.mkdtemp()) @@ -404,14 +561,12 @@ class TestRepo(unittest.TestCase): git_repo = GitRepo(InputOutput(), None, None) - git_repo.commit(fnames=[str(fname)]) + commit_result = git_repo.commit(fnames=[str(fname)]) + self.assertIsNone(commit_result) + @unittest.skipIf(platform.system() == "Windows", "Git hook execution differs on Windows") def test_git_commit_verify(self): """Test that git_commit_verify controls whether --no-verify is passed to git commit""" - # Skip on Windows as hook execution works differently - if platform.system() == "Windows": - return - with GitTemporaryDirectory(): # Create a new repo raw_repo = git.Repo()