From 35e8d62b3a046c8e2edd1279f1aba962e1ea1e7c Mon Sep 17 00:00:00 2001 From: Dan Childers Date: Thu, 23 Jul 2026 15:14:47 -0400 Subject: [PATCH 1/4] fix: use origin remote when fork is not yet configured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit prepare_workspace() defaulted to remote="fork" unconditionally, but on first implementation (before PR creation) the clone only has "origin". This caused git fetch fork to fail, falling back to recreation which also failed due to missing fork_owner/fork_repo state — triggering the retry loop. Now determines the effective remote based on whether fork state exists, and raises directly on origin sync failures instead of attempting the impossible fork recreation path. Co-Authored-By: Claude Opus 4.6 --- src/forge/workflow/nodes/workspace_setup.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/forge/workflow/nodes/workspace_setup.py b/src/forge/workflow/nodes/workspace_setup.py index 190a5b07..4e28c24d 100644 --- a/src/forge/workflow/nodes/workspace_setup.py +++ b/src/forge/workflow/nodes/workspace_setup.py @@ -91,6 +91,9 @@ def prepare_workspace( fork_repo = state.get("fork_repo", "") ticket_key = state["ticket_key"] + has_fork = bool(fork_owner and fork_repo) + effective_remote = remote if has_fork else "origin" + if workspace_path and Path(workspace_path).exists(): workspace = Workspace( path=Path(workspace_path), @@ -100,8 +103,10 @@ def prepare_workspace( ) git = GitOperations(workspace) try: - git.pull_rebase(remote=remote) + git.pull_rebase(remote=effective_remote) except Exception as e: + if not has_fork: + raise logger.warning( "Workspace sync failed for %s; recreating workspace from fork: %s", ticket_key, @@ -117,6 +122,12 @@ def prepare_workspace( ) return workspace_path, git + if not has_fork: + raise ValueError( + f"Cannot prepare workspace for {ticket_key}: " + "workspace path does not exist and fork is not configured" + ) + # Workspace is missing — recreate from fork branch. return _recreate_workspace_from_fork( ticket_key=ticket_key, From 230d2cf048bbf6fb716370545f0c94d03db41bbd Mon Sep 17 00:00:00 2001 From: Dan Childers Date: Thu, 23 Jul 2026 15:21:37 -0400 Subject: [PATCH 2/4] test: cover prepare_workspace fork remote fallback behavior Three new tests for the no-fork-state paths: - uses "origin" remote when fork_owner/fork_repo are not set - raises directly on origin sync failure instead of attempting fork recreation - raises ValueError when workspace is missing and fork is not configured Co-Authored-By: Claude Opus 4.6 --- .../workflow/nodes/test_workspace_setup.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/unit/workflow/nodes/test_workspace_setup.py b/tests/unit/workflow/nodes/test_workspace_setup.py index 3e42cd68..4064bb86 100644 --- a/tests/unit/workflow/nodes/test_workspace_setup.py +++ b/tests/unit/workflow/nodes/test_workspace_setup.py @@ -319,3 +319,63 @@ def test_sync_failure_recreates_workspace_from_fork(self, tmp_path): new_git.clone.assert_called_once() new_git.add_fork_remote.assert_called_once_with("forge-bot", "repo") new_git.checkout_branch.assert_called_once_with("forge/test-123", remote="fork") + + def test_no_fork_state_uses_origin_remote(self, tmp_path): + """Without fork_owner/fork_repo, pull_rebase should use 'origin'.""" + workspace_path = tmp_path / "forge-TEST-200-org-repo" + workspace_path.mkdir() + + state = create_initial_feature_state( + ticket_key="TEST-200", + current_repo="org/repo", + workspace_path=str(workspace_path), + context={"branch_name": "forge/test-200"}, + ) + + mock_git = MagicMock() + + with patch( + "forge.workflow.nodes.workspace_setup.GitOperations", + return_value=mock_git, + ): + result_path, result_git = prepare_workspace(state) + + assert result_path == str(workspace_path) + assert result_git is mock_git + mock_git.pull_rebase.assert_called_once_with(remote="origin") + + def test_no_fork_state_sync_failure_raises(self, tmp_path): + """Without fork state, origin sync failures raise instead of attempting fork recreation.""" + workspace_path = tmp_path / "forge-TEST-201-org-repo" + workspace_path.mkdir() + + state = create_initial_feature_state( + ticket_key="TEST-201", + current_repo="org/repo", + workspace_path=str(workspace_path), + context={"branch_name": "forge/test-201"}, + ) + + mock_git = MagicMock() + mock_git.pull_rebase.side_effect = RuntimeError("origin unreachable") + + with ( + patch( + "forge.workflow.nodes.workspace_setup.GitOperations", + return_value=mock_git, + ), + pytest.raises(RuntimeError, match="origin unreachable"), + ): + prepare_workspace(state) + + def test_no_fork_missing_workspace_raises(self): + """Without fork state and no workspace on disk, raises ValueError.""" + state = create_initial_feature_state( + ticket_key="TEST-202", + current_repo="org/repo", + workspace_path="/nonexistent/path", + context={"branch_name": "forge/test-202"}, + ) + + with pytest.raises(ValueError, match="fork is not configured"): + prepare_workspace(state) From 1ddf9549de6c6be164a97454b22737089e2f379f Mon Sep 17 00:00:00 2001 From: Dan Childers Date: Fri, 24 Jul 2026 10:44:00 -0400 Subject: [PATCH 3/4] fix: push_to_fork falls back to origin when fork remote is missing prepare_workspace was fixed to use origin for fetch/sync, but push_to_fork still hardcoded the "fork" remote unconditionally. On first implementation (before PR creation), the workspace only has "origin", so the push failed with "'fork' does not appear to be a git repository". Now checks which remotes exist and uses origin as fallback, matching the prepare_workspace fix. Co-Authored-By: Claude Opus 4.6 --- src/forge/workspace/git_ops.py | 13 +++-- tests/unit/workspace/test_push_to_fork.py | 63 +++++++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 tests/unit/workspace/test_push_to_fork.py diff --git a/src/forge/workspace/git_ops.py b/src/forge/workspace/git_ops.py index c78a1a78..fb90ed6c 100644 --- a/src/forge/workspace/git_ops.py +++ b/src/forge/workspace/git_ops.py @@ -159,17 +159,24 @@ def add_fork_remote(self, fork_owner: str, fork_repo: str) -> None: self._run_git("fetch", "fork", check=False) def push_to_fork(self, force: bool = False) -> None: - """Push the current branch to the fork remote. + """Push the current branch to the fork remote, falling back to origin. Args: force: Force push (use with caution). """ - args = ["push", "-u", "fork", self.workspace.branch_name] + remote = self._effective_push_remote() + args = ["push", "-u", remote, self.workspace.branch_name] if force: args.insert(1, "--force") self._run_git(*args) - logger.info(f"Pushed branch {self.workspace.branch_name} to fork") + logger.info(f"Pushed branch {self.workspace.branch_name} to {remote}") + + def _effective_push_remote(self) -> str: + """Return 'fork' if that remote exists, otherwise 'origin'.""" + result = self._run_git("remote", check=False) + remotes = result.stdout.split() + return "fork" if "fork" in remotes else "origin" def create_branch(self, base_branch: str = "main") -> None: """Create and checkout a new branch. diff --git a/tests/unit/workspace/test_push_to_fork.py b/tests/unit/workspace/test_push_to_fork.py new file mode 100644 index 00000000..b834344a --- /dev/null +++ b/tests/unit/workspace/test_push_to_fork.py @@ -0,0 +1,63 @@ +"""Tests for GitOperations.push_to_fork remote fallback behavior.""" + +from pathlib import Path +from subprocess import CompletedProcess +from unittest.mock import MagicMock, patch + +from forge.workspace.git_ops import GitOperations +from forge.workspace.manager import Workspace + + +def _git_ops(tmp_path: Path) -> GitOperations: + settings = MagicMock() + settings.github_token.get_secret_value.return_value = "ghp_fake" + workspace = Workspace( + path=tmp_path / "repo", + repo_name="org/repo", + branch_name="forge/test-123", + ticket_key="TEST-123", + ) + with patch("forge.workspace.git_ops.get_settings", return_value=settings): + return GitOperations(workspace) + + +def test_push_to_fork_uses_fork_when_available(tmp_path): + git = _git_ops(tmp_path) + + def fake_run_git(*args, **kwargs): + if args == ("remote",): + return CompletedProcess(args=[], returncode=0, stdout="origin\nfork\n") + return CompletedProcess(args=[], returncode=0, stdout="") + + with patch.object(git, "_run_git", side_effect=fake_run_git) as run_git: + git.push_to_fork() + + run_git.assert_any_call("push", "-u", "fork", "forge/test-123") + + +def test_push_to_fork_falls_back_to_origin(tmp_path): + git = _git_ops(tmp_path) + + def fake_run_git(*args, **kwargs): + if args == ("remote",): + return CompletedProcess(args=[], returncode=0, stdout="origin\n") + return CompletedProcess(args=[], returncode=0, stdout="") + + with patch.object(git, "_run_git", side_effect=fake_run_git) as run_git: + git.push_to_fork() + + run_git.assert_any_call("push", "-u", "origin", "forge/test-123") + + +def test_push_to_fork_force_flag(tmp_path): + git = _git_ops(tmp_path) + + def fake_run_git(*args, **kwargs): + if args == ("remote",): + return CompletedProcess(args=[], returncode=0, stdout="origin\nfork\n") + return CompletedProcess(args=[], returncode=0, stdout="") + + with patch.object(git, "_run_git", side_effect=fake_run_git) as run_git: + git.push_to_fork(force=True) + + run_git.assert_any_call("push", "--force", "-u", "fork", "forge/test-123") From 02c3089498ac0e02110278292dd05c250ff87351 Mon Sep 17 00:00:00 2001 From: Dan Childers Date: Fri, 24 Jul 2026 10:50:01 -0400 Subject: [PATCH 4/4] fix: guard force-push fallback and harden push_to_fork tests Address review findings from roborev job 74: - Refuse to force-push when fork remote is absent (prevents accidentally overwriting upstream history via origin fallback) - Log a warning when falling back to origin so operators can detect misconfiguration - Tests now verify check=False is passed to the git-remote call - Added test for force-push-without-fork guard Co-Authored-By: Claude Opus 4.6 --- src/forge/workspace/git_ops.py | 10 +++++ tests/unit/workspace/test_push_to_fork.py | 50 +++++++++++++++-------- 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/src/forge/workspace/git_ops.py b/src/forge/workspace/git_ops.py index fb90ed6c..9f5d2f5a 100644 --- a/src/forge/workspace/git_ops.py +++ b/src/forge/workspace/git_ops.py @@ -161,10 +161,18 @@ def add_fork_remote(self, fork_owner: str, fork_repo: str) -> None: def push_to_fork(self, force: bool = False) -> None: """Push the current branch to the fork remote, falling back to origin. + When force=True the fork remote is required — force-pushing to origin + could overwrite upstream history, so we raise instead of falling back. + Args: force: Force push (use with caution). """ remote = self._effective_push_remote() + if force and remote != "fork": + raise GitError( + "Refusing to force-push: 'fork' remote is not configured. " + "Force-pushing to origin could overwrite upstream history." + ) args = ["push", "-u", remote, self.workspace.branch_name] if force: args.insert(1, "--force") @@ -176,6 +184,8 @@ def _effective_push_remote(self) -> str: """Return 'fork' if that remote exists, otherwise 'origin'.""" result = self._run_git("remote", check=False) remotes = result.stdout.split() + if "fork" not in remotes: + logger.warning("Fork remote not configured; falling back to origin for push") return "fork" if "fork" in remotes else "origin" def create_branch(self, base_branch: str = "main") -> None: diff --git a/tests/unit/workspace/test_push_to_fork.py b/tests/unit/workspace/test_push_to_fork.py index b834344a..63a61135 100644 --- a/tests/unit/workspace/test_push_to_fork.py +++ b/tests/unit/workspace/test_push_to_fork.py @@ -4,7 +4,9 @@ from subprocess import CompletedProcess from unittest.mock import MagicMock, patch -from forge.workspace.git_ops import GitOperations +import pytest + +from forge.workspace.git_ops import GitError, GitOperations from forge.workspace.manager import Workspace @@ -21,15 +23,24 @@ def _git_ops(tmp_path: Path) -> GitOperations: return GitOperations(workspace) -def test_push_to_fork_uses_fork_when_available(tmp_path): - git = _git_ops(tmp_path) +def _fake_run_git_with_remotes(*remote_names: str): + stdout = "\n".join(remote_names) + "\n" - def fake_run_git(*args, **kwargs): + def fake(*args, **kwargs): if args == ("remote",): - return CompletedProcess(args=[], returncode=0, stdout="origin\nfork\n") + assert kwargs.get("check") is False + return CompletedProcess(args=[], returncode=0, stdout=stdout) return CompletedProcess(args=[], returncode=0, stdout="") - with patch.object(git, "_run_git", side_effect=fake_run_git) as run_git: + return fake + + +def test_push_to_fork_uses_fork_when_available(tmp_path): + git = _git_ops(tmp_path) + + with patch.object( + git, "_run_git", side_effect=_fake_run_git_with_remotes("origin", "fork") + ) as run_git: git.push_to_fork() run_git.assert_any_call("push", "-u", "fork", "forge/test-123") @@ -38,12 +49,7 @@ def fake_run_git(*args, **kwargs): def test_push_to_fork_falls_back_to_origin(tmp_path): git = _git_ops(tmp_path) - def fake_run_git(*args, **kwargs): - if args == ("remote",): - return CompletedProcess(args=[], returncode=0, stdout="origin\n") - return CompletedProcess(args=[], returncode=0, stdout="") - - with patch.object(git, "_run_git", side_effect=fake_run_git) as run_git: + with patch.object(git, "_run_git", side_effect=_fake_run_git_with_remotes("origin")) as run_git: git.push_to_fork() run_git.assert_any_call("push", "-u", "origin", "forge/test-123") @@ -52,12 +58,20 @@ def fake_run_git(*args, **kwargs): def test_push_to_fork_force_flag(tmp_path): git = _git_ops(tmp_path) - def fake_run_git(*args, **kwargs): - if args == ("remote",): - return CompletedProcess(args=[], returncode=0, stdout="origin\nfork\n") - return CompletedProcess(args=[], returncode=0, stdout="") - - with patch.object(git, "_run_git", side_effect=fake_run_git) as run_git: + with patch.object( + git, "_run_git", side_effect=_fake_run_git_with_remotes("origin", "fork") + ) as run_git: git.push_to_fork(force=True) run_git.assert_any_call("push", "--force", "-u", "fork", "forge/test-123") + + +def test_push_to_fork_force_without_fork_raises(tmp_path): + """Force-pushing without a fork remote must raise to prevent overwriting upstream.""" + git = _git_ops(tmp_path) + + with ( + patch.object(git, "_run_git", side_effect=_fake_run_git_with_remotes("origin")), + pytest.raises(GitError, match="Refusing to force-push"), + ): + git.push_to_fork(force=True)