From a641c7e6e742a5325f53396f1e66bb2db788b868 Mon Sep 17 00:00:00 2001 From: Hiroshi Nishio Date: Wed, 25 Feb 2026 15:23:25 -0800 Subject: [PATCH 1/2] Fix EFS clone HEAD pointing to wrong branch after fetch+reset When .git already existed on EFS, git fetch + reset --hard FETCH_HEAD updated the working tree but left HEAD pointing to the original branch (e.g. master). Added git checkout -B to update HEAD to the correct target branch. --- services/git/git_clone_to_efs.py | 4 ++++ services/git/test_git_clone_to_efs.py | 13 ++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/services/git/git_clone_to_efs.py b/services/git/git_clone_to_efs.py index e8237011b..7dbbe1f7e 100644 --- a/services/git/git_clone_to_efs.py +++ b/services/git/git_clone_to_efs.py @@ -64,6 +64,10 @@ async def git_clone_to_efs(efs_dir: str, clone_url: str, branch: str): await run_subprocess_async( ["git", "reset", "--hard", "FETCH_HEAD"], efs_dir ) + # Update HEAD to point to the correct branch (reset doesn't change it) + await run_subprocess_async( + ["git", "checkout", "-B", branch, "FETCH_HEAD"], efs_dir + ) return efs_dir # Always use init + fetch + checkout instead of clone diff --git a/services/git/test_git_clone_to_efs.py b/services/git/test_git_clone_to_efs.py index 5ae3b1269..f8953d23a 100644 --- a/services/git/test_git_clone_to_efs.py +++ b/services/git/test_git_clone_to_efs.py @@ -140,8 +140,8 @@ async def test_git_clone_updates_origin_url_before_fetch(mock_os_makedirs): result = await git_clone_to_efs("/mnt/efs/repo", fresh_url, "main") assert result == "/mnt/efs/repo" - # safe.directory + get-url + set-url + fetch + reset - assert mock_run.call_count == 5 + # safe.directory + get-url + set-url + fetch + reset + checkout -B + assert mock_run.call_count == 6 calls = [call[0][0] for call in mock_run.call_args_list] assert calls[0] == [ "git", @@ -155,6 +155,7 @@ async def test_git_clone_updates_origin_url_before_fetch(mock_os_makedirs): assert calls[2] == ["git", "remote", "set-url", "origin", fresh_url] assert calls[3] == ["git", "fetch", "--depth", "1", "origin", "main"] assert calls[4] == ["git", "reset", "--hard", "FETCH_HEAD"] + assert calls[5] == ["git", "checkout", "-B", "main", "FETCH_HEAD"] @pytest.mark.asyncio @@ -166,13 +167,14 @@ async def test_git_clone_adds_origin_when_missing(mock_os_makedirs): with patch( "services.git.git_clone_to_efs.run_subprocess_async", new_callable=AsyncMock ) as mock_run: - # safe.directory succeeds, get-url fails (no origin), add succeeds, fetch succeeds, reset succeeds + # safe.directory succeeds, get-url fails (no origin), add succeeds, fetch succeeds, reset succeeds, checkout succeeds mock_run.side_effect = [ (0, ""), (1, "error: No such remote 'origin'"), (0, ""), (0, ""), (0, ""), + (0, ""), ] clone_url = "https://github.com/owner/repo.git" @@ -180,8 +182,8 @@ async def test_git_clone_adds_origin_when_missing(mock_os_makedirs): result = await git_clone_to_efs("/mnt/efs/repo", clone_url, "main") assert result == "/mnt/efs/repo" - # safe.directory + get-url + add + fetch + reset - assert mock_run.call_count == 5 + # safe.directory + get-url + add + fetch + reset + checkout -B + assert mock_run.call_count == 6 calls = [call[0][0] for call in mock_run.call_args_list] assert calls[0] == [ "git", @@ -195,3 +197,4 @@ async def test_git_clone_adds_origin_when_missing(mock_os_makedirs): assert calls[2] == ["git", "remote", "add", "origin", clone_url] assert calls[3] == ["git", "fetch", "--depth", "1", "origin", "main"] assert calls[4] == ["git", "reset", "--hard", "FETCH_HEAD"] + assert calls[5] == ["git", "checkout", "-B", "main", "FETCH_HEAD"] From 370485e1a781235d0cb2b916d84967e8c55a2818 Mon Sep 17 00:00:00 2001 From: Hiroshi Nishio Date: Wed, 25 Feb 2026 15:51:38 -0800 Subject: [PATCH 2/2] Fix EFS clone HEAD pointing to wrong branch after fetch+reset When .git already existed on EFS, git fetch + reset --hard FETCH_HEAD updated the working tree but left HEAD pointing to the original branch (e.g. master instead of release/20260408). Now reads .git/HEAD via read_local_file and runs git checkout -B only when the branch actually changed, avoiding unnecessary work when the branch is the same. --- services/git/git_clone_to_efs.py | 19 ++++--- services/git/test_git_clone_to_efs.py | 82 ++++++++++++++++++++------- 2 files changed, 74 insertions(+), 27 deletions(-) diff --git a/services/git/git_clone_to_efs.py b/services/git/git_clone_to_efs.py index 7dbbe1f7e..db3b2ceb5 100644 --- a/services/git/git_clone_to_efs.py +++ b/services/git/git_clone_to_efs.py @@ -4,6 +4,7 @@ from services.git.resolve_git_locks import resolve_git_locks from utils.command.run_subprocess_async import run_subprocess_async from utils.error.handle_exceptions import handle_exceptions +from utils.files.read_local_file import read_local_file from utils.logging.logging_config import logger # clone_tasks example: @@ -55,19 +56,21 @@ async def git_clone_to_efs(efs_dir: str, clone_url: str, branch: str): ["git", "fetch", "--depth", "1", "origin", branch], efs_dir ) if returncode == 0: - fetch_head = os.path.join(efs_git_dir, "FETCH_HEAD") - if os.path.exists(fetch_head): - with open(fetch_head, encoding="utf-8") as f: - logger.info("FETCH_HEAD: %s", f.read().strip()) + fetch_head_content = read_local_file("FETCH_HEAD", efs_git_dir) + if fetch_head_content: + logger.info("FETCH_HEAD: %s", fetch_head_content.strip()) else: logger.warning("FETCH_HEAD missing despite fetch success") await run_subprocess_async( ["git", "reset", "--hard", "FETCH_HEAD"], efs_dir ) - # Update HEAD to point to the correct branch (reset doesn't change it) - await run_subprocess_async( - ["git", "checkout", "-B", branch, "FETCH_HEAD"], efs_dir - ) + # Switch HEAD to the correct branch if it changed (e.g. master → release/20260408) + head_content = read_local_file("HEAD", efs_git_dir) + current_ref = head_content.strip() if head_content else "" + if current_ref != f"ref: refs/heads/{branch}": + await run_subprocess_async( + ["git", "checkout", "-B", branch, "FETCH_HEAD"], efs_dir + ) return efs_dir # Always use init + fetch + checkout instead of clone diff --git a/services/git/test_git_clone_to_efs.py b/services/git/test_git_clone_to_efs.py index f8953d23a..ff0e0ba68 100644 --- a/services/git/test_git_clone_to_efs.py +++ b/services/git/test_git_clone_to_efs.py @@ -1,6 +1,6 @@ # pylint: disable=unused-argument # pyright: reportUnusedVariable=false -from unittest.mock import patch, AsyncMock +from unittest.mock import AsyncMock, patch import pytest @@ -136,12 +136,20 @@ async def test_git_clone_updates_origin_url_before_fetch(mock_os_makedirs): mock_run.return_value = (0, "") fresh_url = "https://x-access-token:fresh_token@github.com/owner/repo.git" - with patch("builtins.open", create=True): + # FETCH_HEAD exists, HEAD already on main → checkout -B skipped + with patch( + "services.git.git_clone_to_efs.read_local_file", + side_effect=lambda f, _d: ( + "abc123\tbranch 'main'" + if f == "FETCH_HEAD" + else "ref: refs/heads/main" + ), + ): result = await git_clone_to_efs("/mnt/efs/repo", fresh_url, "main") assert result == "/mnt/efs/repo" - # safe.directory + get-url + set-url + fetch + reset + checkout -B - assert mock_run.call_count == 6 + # safe.directory + get-url + set-url + fetch + reset (no checkout -B, branch unchanged) + assert mock_run.call_count == 5 calls = [call[0][0] for call in mock_run.call_args_list] assert calls[0] == [ "git", @@ -155,7 +163,45 @@ async def test_git_clone_updates_origin_url_before_fetch(mock_os_makedirs): assert calls[2] == ["git", "remote", "set-url", "origin", fresh_url] assert calls[3] == ["git", "fetch", "--depth", "1", "origin", "main"] assert calls[4] == ["git", "reset", "--hard", "FETCH_HEAD"] - assert calls[5] == ["git", "checkout", "-B", "main", "FETCH_HEAD"] + + +@pytest.mark.asyncio +async def test_git_clone_switches_branch_when_target_changes(mock_os_makedirs): + """When target branch changed (e.g. master → release/20260408), checkout -B updates HEAD.""" + with patch("services.git.git_clone_to_efs.os.path.exists") as mock_exists: + mock_exists.return_value = True + + with patch( + "services.git.git_clone_to_efs.run_subprocess_async", new_callable=AsyncMock + ) as mock_run: + mock_run.return_value = (0, "") + + clone_url = "https://github.com/owner/repo.git" + # HEAD on master, but fetching release/20260408 → checkout -B runs + with patch( + "services.git.git_clone_to_efs.read_local_file", + side_effect=lambda f, _d: ( + "abc123\tbranch 'release/20260408'" + if f == "FETCH_HEAD" + else "ref: refs/heads/master" + ), + ): + result = await git_clone_to_efs( + "/mnt/efs/repo", clone_url, "release/20260408" + ) + + assert result == "/mnt/efs/repo" + # safe.directory + get-url + set-url + fetch + reset + checkout -B + assert mock_run.call_count == 6 + calls = [call[0][0] for call in mock_run.call_args_list] + assert calls[4] == ["git", "reset", "--hard", "FETCH_HEAD"] + assert calls[5] == [ + "git", + "checkout", + "-B", + "release/20260408", + "FETCH_HEAD", + ] @pytest.mark.asyncio @@ -167,34 +213,32 @@ async def test_git_clone_adds_origin_when_missing(mock_os_makedirs): with patch( "services.git.git_clone_to_efs.run_subprocess_async", new_callable=AsyncMock ) as mock_run: - # safe.directory succeeds, get-url fails (no origin), add succeeds, fetch succeeds, reset succeeds, checkout succeeds + # safe.directory succeeds, get-url fails (no origin), add succeeds, fetch succeeds, reset succeeds mock_run.side_effect = [ (0, ""), (1, "error: No such remote 'origin'"), (0, ""), (0, ""), (0, ""), - (0, ""), ] clone_url = "https://github.com/owner/repo.git" - with patch("builtins.open", create=True): + # HEAD already on main → checkout -B skipped + with patch( + "services.git.git_clone_to_efs.read_local_file", + side_effect=lambda f, _d: ( + "abc123\tbranch 'main'" + if f == "FETCH_HEAD" + else "ref: refs/heads/main" + ), + ): result = await git_clone_to_efs("/mnt/efs/repo", clone_url, "main") assert result == "/mnt/efs/repo" - # safe.directory + get-url + add + fetch + reset + checkout -B - assert mock_run.call_count == 6 + # safe.directory + get-url + add + fetch + reset (no checkout -B, branch unchanged) + assert mock_run.call_count == 5 calls = [call[0][0] for call in mock_run.call_args_list] - assert calls[0] == [ - "git", - "config", - "--global", - "--add", - "safe.directory", - "/mnt/efs/repo", - ] assert calls[1] == ["git", "remote", "get-url", "origin"] assert calls[2] == ["git", "remote", "add", "origin", clone_url] assert calls[3] == ["git", "fetch", "--depth", "1", "origin", "main"] assert calls[4] == ["git", "reset", "--hard", "FETCH_HEAD"] - assert calls[5] == ["git", "checkout", "-B", "main", "FETCH_HEAD"]