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, diff --git a/src/forge/workspace/git_ops.py b/src/forge/workspace/git_ops.py index c78a1a78..9f5d2f5a 100644 --- a/src/forge/workspace/git_ops.py +++ b/src/forge/workspace/git_ops.py @@ -159,17 +159,34 @@ 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. + + 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). """ - args = ["push", "-u", "fork", self.workspace.branch_name] + 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") 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() + 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: """Create and checkout a new branch. 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) 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..63a61135 --- /dev/null +++ b/tests/unit/workspace/test_push_to_fork.py @@ -0,0 +1,77 @@ +"""Tests for GitOperations.push_to_fork remote fallback behavior.""" + +from pathlib import Path +from subprocess import CompletedProcess +from unittest.mock import MagicMock, patch + +import pytest + +from forge.workspace.git_ops import GitError, 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 _fake_run_git_with_remotes(*remote_names: str): + stdout = "\n".join(remote_names) + "\n" + + def fake(*args, **kwargs): + if args == ("remote",): + assert kwargs.get("check") is False + return CompletedProcess(args=[], returncode=0, stdout=stdout) + return CompletedProcess(args=[], returncode=0, stdout="") + + 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") + + +def test_push_to_fork_falls_back_to_origin(tmp_path): + git = _git_ops(tmp_path) + + 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") + + +def test_push_to_fork_force_flag(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(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)