Infer base repository from parent branch in retarget/restack

git_machete/client/with_code_hosting.py

@@ -433,7 +433,8 @@ def create_pull_request(
     def restack_pull_request(self, *, opt_update_related_descriptions: bool) -> None:
         spec = self.code_hosting_spec
         head = self._git.get_current_branch()
-        _, org_repo_remote = self._init_code_hosting_client(branch_used_for_tracking_data=head)
+        _, org_repo_remote = self._init_code_hosting_client(
+            branch_used_for_tracking_data=head, base_branch_used_for_tracking_data=self.parent_of(head))
 
         pr: Optional[PullRequest] = self.__get_sole_pull_request_for_head(head, ignore_if_missing=False)
         assert pr is not None
@@ -538,7 +539,8 @@ def retarget_pull_request(self, *, opt_branch: Optional[LocalBranchShortName],
         head: ManagedBranchName = self.expect_in_managed_branches(opt_branch or self._git.get_current_branch())
         spec = self.code_hosting_spec
         if self.__code_hosting_client is None:
-            self._init_code_hosting_client(branch_used_for_tracking_data=head)
+            self._init_code_hosting_client(
+                branch_used_for_tracking_data=head, base_branch_used_for_tracking_data=self.parent_of(head))
 
         pr: Optional[PullRequest] = self.__get_sole_pull_request_for_head(
             head, ignore_if_missing=opt_ignore_if_missing)
@@ -599,16 +601,23 @@ def __derive_org_repo_and_remote(
             domain: str,
             branch_used_for_tracking_data: Optional[LocalBranchShortName] = None,
             *,
-            is_base: bool = False
+            is_base: bool = False,
+            fall_back_org_repo_to_head: bool = False
     ) -> OrganizationAndRepositoryAndRemote:
         spec = self.code_hosting_spec
         keys = spec.git_config_keys
         if is_base:
             remote_key, org_key, repo_key = keys.base_remote, keys.base_organization, keys.base_repository
-            # The base remote falls back to the (non-base) remote; the base organization/repository have no such fallback.
+            # The base remote falls back to the (non-base) remote; the base organization/repository have no such fallback
+            # by default (`create_pull_request` relies on that to detect a fork base). PR-reading/-modifying commands, however,
+            # opt into the fallback via `fall_back_org_repo_to_head`, so that a setup which pins only the head
+            # `organization`/`repository` (and no base* keys) keeps working when the client is resolved against the base.
             remote_from_config = self._config.code_hosting_base_remote(keys) or self._config.code_hosting_remote(keys)
             org_from_config = self._config.code_hosting_base_organization(keys)
             repo_from_config = self._config.code_hosting_base_repository(keys)
+            if fall_back_org_repo_to_head:
+                org_from_config = org_from_config or self._config.code_hosting_organization(keys)
+                repo_from_config = repo_from_config or self._config.code_hosting_repository(keys)
         else:
             remote_key, org_key, repo_key = keys.remote, keys.organization, keys.repository
             remote_from_config = self._config.code_hosting_remote(keys)
@@ -691,16 +700,40 @@ def __derive_org_repo_and_remote(
             f'{spec.git_config_keys.for_locating_repo_message()}\n')
 
     def _init_code_hosting_client(self,
-                                  branch_used_for_tracking_data: Optional[LocalBranchShortName] = None
+                                  branch_used_for_tracking_data: Optional[LocalBranchShortName] = None,
+                                  base_branch_used_for_tracking_data: Optional[LocalBranchShortName] = None
                                   ) -> Tuple[str, OrganizationAndRepositoryAndRemote]:
         if self.__code_hosting_client is not None:
             raise UnexpectedMacheteException("Code hosting client has already been initialized.")
         domain = self.__derive_code_hosting_domain()
-        org_repo_remote = self.__derive_org_repo_and_remote(
+        # The returned org/repo/remote identifies the *head* repository (the one that holds the branches), which callers use
+        # to fetch/push. The code hosting client, however, must talk to the repository that *hosts* the PRs, i.e. the base
+        # repository - in a fork workflow the base (upstream) repository differs from the head (fork) one. When a base branch
+        # is given, the base repository is inferred from that branch's tracking remote (exactly like `create_pull_request`),
+        # so retarget/restack query the right repository even with no base* config keys. When no base branch is given, the base
+        # repository resolves to the head one, so this is a no-op for the common (non-fork) case.
+        head_org_repo_remote = self.__derive_org_repo_and_remote(
             domain=domain, branch_used_for_tracking_data=branch_used_for_tracking_data)
+        base_org_repo_remote = head_org_repo_remote
+        if base_branch_used_for_tracking_data is not None:
+            keys = self.code_hosting_spec.git_config_keys
+            base_config_present = (
+                self._config.code_hosting_base_remote(keys) is not None or
+                self._config.code_hosting_base_organization(keys) is not None or
+                self._config.code_hosting_base_repository(keys) is not None)
+            try:
+                base_org_repo_remote = self.__derive_org_repo_and_remote(
+                    domain=domain, branch_used_for_tracking_data=base_branch_used_for_tracking_data,
+                    is_base=True, fall_back_org_repo_to_head=True)
+            except MacheteException:
+                # The base repository could not be resolved unambiguously (for example the base branch has no tracking data
+                # among several candidate remotes). With no explicit base* config to honor, fall back to the head repository,
+                # preserving the pre-inference behavior; with base* keys set, the misconfiguration should surface.
+                if base_config_present:
+                    raise
         self.code_hosting_client = self.code_hosting_spec.create_client(
-            domain=domain, organization=org_repo_remote.organization, repository=org_repo_remote.repository)
-        return domain, org_repo_remote
+            domain=domain, organization=base_org_repo_remote.organization, repository=base_org_repo_remote.repository)
+        return domain, head_org_repo_remote
 
     START_GIT_MACHETE_GENERATED_COMMENT = '<!-- start git-machete generated -->'
     END_GIT_MACHETE_GENERATED_COMMENT = '<!-- end git-machete generated -->'

tests/mockers_github.py

@@ -14,6 +14,7 @@
 
 def mock_pr_json(head: str, base: str, number: int,
                  repo_id: int = 1,
+                 base_repo_id: Optional[int] = None,
                  user: str = 'some_other_user',
                  html_url: str = 'www.github.com',
                  body: Optional[str] = '# Summary',
@@ -23,7 +24,9 @@ def mock_pr_json(head: str, base: str, number: int,
     return {
         'head': {'ref': head, 'repo': {'id': repo_id}},
         'user': {'login': user},
-        'base': {'ref': base},
+        # `base.repo.id` records which repository hosts the PR; `None` (the default) means "served for any repository",
+        # so the vast majority of tests that don't care about fork/base targeting keep working unchanged.
+        'base': {'ref': base, 'repo': {'id': base_repo_id}},
         'number': str(number),
         'html_url': html_url,
         'title': 'PR title',
@@ -113,21 +116,35 @@ def url_with_query_params(**new_params: Any) -> str:
         new_query_string: str = urlencode({**query_params, **new_params})
         return parsed_url._replace(query=new_query_string).geturl()
 
+    def find_repo_id_by_org_and_repo(org: str, repo: str) -> Optional[int]:
+        for repo_id, repo_data in github_api_state.repositories.items():
+            if repo_data['owner']['login'] == org and repo_data['name'] == repo:
+                return repo_id
+        return None
+
     def handle_get() -> "MockAPIResponse":
         if url_path_matches('/repositories/[0-9]+'):
             repo_no = int(url_segments[-1])
             if repo_no in github_api_state.repositories:
                 return MockAPIResponse(HTTPStatus.OK, github_api_state.repositories[repo_no])
             raise error_404()
         elif url_path_matches('/repos/*/*/pulls'):
+            # The `/repos/{org}/{repo}/pulls` endpoint lists PRs hosted by the base repository `{org}/{repo}`,
+            # so filter out PRs whose base repository (if declared) differs from the one being queried.
+            requested_base_repo_id: Optional[int] = find_repo_id_by_org_and_repo(url_segments[-3], url_segments[-2])
+
+            def hosted_by_requested_repo(pull: Dict[str, Any]) -> bool:
+                base_repo_id = pull['base'].get('repo', {}).get('id')
+                return base_repo_id is None or base_repo_id == requested_base_repo_id
+
             full_head_name: Optional[str] = query_params.get('head')
             if full_head_name:
                 head: str = full_head_name.split(':')[1]
-                prs = github_api_state.get_open_pulls_by_head(head)
+                prs = [pull for pull in github_api_state.get_open_pulls_by_head(head) if hosted_by_requested_repo(pull)]
                 # If no matching PRs are found, the real GitHub returns 200 OK with an empty JSON array - not 404.
                 return MockAPIResponse(HTTPStatus.OK, prs)
             else:
-                pulls = github_api_state.get_open_pulls()
+                pulls = [pull for pull in github_api_state.get_open_pulls() if hosted_by_requested_repo(pull)]
                 page_str = query_params.get('page')
                 page = int(page_str) if page_str else 1
                 per_page = int(query_params['per_page'])

tests/mockers_gitlab.py

@@ -21,6 +21,7 @@
 
 def mock_mr_json(head: str, base: str, number: int,
                  repo_id: int = 1,
+                 base_repo_id: Optional[int] = None,
                  user: str = 'some_other_user',
                  html_url: str = 'www.gitlab.com',
                  body: Optional[str] = '# Summary',
@@ -30,6 +31,9 @@ def mock_mr_json(head: str, base: str, number: int,
     return {
         'source_branch': head,
         'source_project_id': repo_id,
+        # `project_id` records the target project that hosts the MR; `None` (the default) means "served for any project",
+        # so the vast majority of tests that don't care about fork/target targeting keep working unchanged.
+        'project_id': base_repo_id,
         'target_branch': base,
         'author': {'username': user},
         'iid': str(number),
@@ -127,6 +131,15 @@ def url_with_query_params(**new_params: Any) -> str:
         new_query_string: str = urlencode({**query_params, **new_params})
         return parsed_url._replace(query=new_query_string).geturl()
 
+    def find_project_id(id_or_path: str) -> Optional[int]:
+        if id_or_path.isdigit():
+            return int(id_or_path)
+        full_path = urllib.parse.unquote(id_or_path)
+        for project_id, project in gitlab_api_state.projects.items():
+            if project['namespace']['full_path'] == full_path:
+                return project_id
+        return None
+
     def handle_get() -> "MockAPIResponse":
         if url_path_matches('/projects/[0-9]+'):
             repo_no = int(url_segments[-1])
@@ -140,13 +153,21 @@ def handle_get() -> "MockAPIResponse":
                     return MockAPIResponse(HTTPStatus.OK, project)
             raise error_404()
         elif url_path_matches('/projects/*/merge_requests'):
+            # The `/projects/{id}/merge_requests` endpoint lists MRs hosted by the target project `{id}`,
+            # so filter out MRs whose target project (if declared) differs from the one being queried.
+            requested_target_project_id: Optional[int] = find_project_id(url_segments[-2])
+
+            def hosted_by_requested_project(mr: Dict[str, Any]) -> bool:
+                target_project_id = mr.get('project_id')
+                return target_project_id is None or target_project_id == requested_target_project_id
+
             head: Optional[str] = query_params.get('source_branch')
             if head:
-                mrs = gitlab_api_state.get_open_mrs_by_head(head)
+                mrs = [mr for mr in gitlab_api_state.get_open_mrs_by_head(head) if hosted_by_requested_project(mr)]
                 # If no matching MRs are found, the real GitLab returns 200 OK with an empty JSON array - not 404.
                 return MockAPIResponse(HTTPStatus.OK, mrs)
             else:
-                mrs = gitlab_api_state.get_open_mrs()
+                mrs = [mr for mr in gitlab_api_state.get_open_mrs() if hosted_by_requested_project(mr)]
                 page_str = query_params.get('page')
                 page = int(page_str) if page_str else 1
                 per_page_str = query_params.get('per_page')

tests/test_github_restack_pr.py

@@ -4,7 +4,8 @@
 
 from tests.base_test import BaseTest
 from tests.cli_runner import assert_failure, assert_success, rewrite_branch_layout_file
-from tests.git_repository import amend_commit, commit, create_repo_with_remote, new_branch, push, reset_to, set_git_config_key
+from tests.git_repository import (add_remote, amend_commit, commit, create_repo, create_repo_with_remote, new_branch, push, reset_to,
+                                  set_git_config_key)
 from tests.mockers import fixed_author_and_committer_date_in_past
 from tests.mockers_code_hosting import mock_from_url
 from tests.mockers_github import MockGitHubAPIState, mock_github_token_for_domain_fake, mock_pr_json, mock_urlopen
@@ -331,3 +332,46 @@ def test_github_restack_pr_branch_diverged_and_older(self, mocker: MockerFixture
         assert pr is not None
         assert pr['draft'] is False
         assert pr['base']['ref'] == 'master'
+
+    def test_github_restack_pr_infers_base_repo_from_parent_tracking(self, mocker: MockerFixture) -> None:
+        # In a fork workflow the PR is hosted by the base (upstream) repository, not the head (fork) repository.
+        # Even with no base* config keys, restack-pr infers the base repository from the parent branch's tracking remote
+        # (just like create-pr does), so it queries the repository that actually hosts the PR.
+        self.patch_symbol(mocker, 'git_machete.github.GitHubToken.for_domain', mock_github_token_for_domain_fake)
+        self.patch_symbol(mocker, 'git_machete.code_hosting.OrganizationAndRepository.from_url', mock_from_url)
+        repositories = {
+            1: {'owner': {'login': 'example-org'}, 'name': 'example-repo',
+                'clone_url': 'https://github.com/example-org/example-repo.git'},
+            2: {'owner': {'login': 'example-org'}, 'name': 'example-repo-1',
+                'clone_url': 'https://github.com/example-org/example-repo-1.git'},
+        }
+        github_api_state = MockGitHubAPIState(
+            repositories,
+            mock_pr_json(number=1, head='feature', base='master', repo_id=1, base_repo_id=2, user='github_user'))
+        self.patch_symbol(mocker, 'urllib.request.urlopen', mock_urlopen(github_api_state))
+
+        # `origin` (-> example-org/example-repo) is the head/fork remote holding the branches, while
+        # `upstream` (-> example-org/example-repo-1) is the base remote that hosts the PR. The parent branch (develop)
+        # tracks `upstream`, so the base repository is inferred from it - no machete.github.base* key is set.
+        create_repo_with_remote()
+        upstream_path = create_repo("remote-1", bare=True, switch_dir_to_new_repo=False)
+        add_remote("upstream", upstream_path)
+
+        new_branch("master")
+        commit()
+        push()
+        new_branch("develop")
+        commit()
+        push(remote="upstream")
+        new_branch("feature")
+        commit()
+        push()
+        rewrite_branch_layout_file("master\n\tdevelop\n\t\tfeature")
+
+        assert_success(
+            ['github', 'restack-pr'],
+            "Switching base branch of PR #1 to develop... OK\n"
+        )
+        pr = github_api_state.get_pull_by_number(1)
+        assert pr is not None
+        assert pr['base']['ref'] == 'develop'

tests/test_github_retarget_pr.py

@@ -487,3 +487,46 @@ def test_github_retarget_pr_root_branch(self, mocker: MockerFixture) -> None:
             "Branch master does not have a parent branch (it is a root) even though there is an open PR #15 to root.\n"
             "Consider modifying the branch layout file (git machete edit) so that master is a child of root."
         )
+
+    def test_github_retarget_pr_infers_base_repo_from_parent_tracking(self, mocker: MockerFixture) -> None:
+        # In a fork workflow the PR is hosted by the base (upstream) repository, not the head (fork) repository.
+        # Even with no base* config keys, retarget-pr infers the base repository from the parent branch's tracking remote
+        # (just like create-pr does), so it queries the repository that actually hosts the PR.
+        self.patch_symbol(mocker, 'git_machete.code_hosting.OrganizationAndRepository.from_url', mock_from_url)
+        self.patch_symbol(mocker, 'git_machete.github.GitHubToken.for_domain', mock_github_token_for_domain_fake)
+        repositories = {
+            1: {'owner': {'login': 'example-org'}, 'name': 'example-repo',
+                'clone_url': 'https://github.com/example-org/example-repo.git'},
+            2: {'owner': {'login': 'example-org'}, 'name': 'example-repo-1',
+                'clone_url': 'https://github.com/example-org/example-repo-1.git'},
+        }
+        github_api_state = MockGitHubAPIState(
+            repositories,
+            mock_pr_json(number=1, head='feature', base='master', repo_id=1, base_repo_id=2, user='github_user'))
+        self.patch_symbol(mocker, 'urllib.request.urlopen', mock_urlopen(github_api_state))
+
+        # `origin` (-> example-org/example-repo) is the head/fork remote holding the branches, while
+        # `upstream` (-> example-org/example-repo-1) is the base remote that hosts the PR. The parent branch (develop)
+        # tracks `upstream`, so the base repository is inferred from it - no machete.github.base* key is set.
+        create_repo_with_remote()
+        upstream_path = create_repo("remote-1", bare=True, switch_dir_to_new_repo=False)
+        add_remote("upstream", upstream_path)
+
+        new_branch("master")
+        commit()
+        push()
+        new_branch("develop")
+        commit()
+        push(remote="upstream")
+        new_branch("feature")
+        commit()
+        push()
+        rewrite_branch_layout_file("master\n\tdevelop\n\t\tfeature")
+
+        assert_success(
+            ['github', 'retarget-pr'],
+            "Switching base branch of PR #1 to develop... OK\n"
+        )
+        pr1 = github_api_state.get_pull_by_number(1)
+        assert pr1 is not None
+        assert pr1['base']['ref'] == 'develop'