From 3bcf4b86ea44d4ce9f72c9bcf285fa94a5667eae Mon Sep 17 00:00:00 2001 From: Caetano Melone Date: Wed, 19 Aug 2026 14:53:51 -0700 Subject: [PATCH 1/3] consolidate shared logic in GitHub routes Signed-off-by: Caetano Melone --- src/hubcast/web/github/routes.py | 541 ++++++++++++++----------------- 1 file changed, 236 insertions(+), 305 deletions(-) diff --git a/src/hubcast/web/github/routes.py b/src/hubcast/web/github/routes.py index 84f87a9..4966cfb 100644 --- a/src/hubcast/web/github/routes.py +++ b/src/hubcast/web/github/routes.py @@ -1,5 +1,6 @@ import logging import re +from collections.abc import Awaitable, Callable from typing import Any from aiohttp.client_exceptions import ClientResponseError @@ -61,6 +62,8 @@ async def dispatch(self, event: sansio.Event, *args: Any, **kwargs: Any) -> None # this check won't linger because resolving issues requires a new commit to be pushed ERROR_CHECK_NAME = "hubcast-error" +NULL_SHA = "0" * 40 + async def report_config_error(gh: GitHubClient, sha: str, exc: RepoConfigError) -> None: """Report a missing/invalid repo config to the user as a failed check.""" @@ -74,92 +77,38 @@ async def report_config_error(gh: GitHubClient, sha: str, exc: RepoConfigError) ) -# ----------------------------------- -# Push Events -# ----------------------------------- -@router.register("push", deleted=False) -async def sync_branch( - event: sansio.Event, - gh: GitHubClient, - gl: GitLabClient, - gl_user: str, - *arg, - **kwargs, -) -> None: - """Sync the git branch referenced to GitLab.""" - src_repo_url = event.data["repository"]["clone_url"] - src_fullname = event.data["repository"]["full_name"] - src_owner, src_repo_name = src_fullname.split("/") - # the commit the push event is referencing - want_sha = event.data["after"] - sync_ref = event.data["ref"] - - update_log_context(ref=sync_ref) - - # skip branches from push events that are also pull requests - if await gh.get_prs(branch=sync_ref): - log.info("Skipped branch sync - branch has open PR") - return +def _pr_sync_branch(pull_request: dict[str, Any]) -> str: + """Return the branch name used on the destination for this PR. - # only refresh config when a default branch push touches .github/hubcast.yml - default_branch = event.data["repository"]["default_branch"] - is_default_branch = sync_ref == f"refs/heads/{default_branch}" - config_changed = gh.repo_config_path in changed_files_from_push(event.data) - try: - repo_config = await get_repo_config( - gh, src_fullname, refresh=is_default_branch and config_changed - ) - except RepoConfigError as exc: - await report_config_error(gh, want_sha, exc) - return + Pull requests coming from forks are pushed as branches in the form of + pr- instead of as their branch name, as conflicts can occur + between multiple source repositories. + """ + src_fullname = pull_request["head"]["repo"]["full_name"] + base_fullname = pull_request["base"]["repo"]["full_name"] + if src_fullname != base_fullname: + return f"pr-{pull_request['number']}" + return pull_request["head"]["ref"] - dest_fullname = repo_config.dest_fullname - dest_remote_url = f"{gl.instance_url}/{dest_fullname}.git" - head_commit = event.data.get("head_commit") - commit_msg = head_commit["message"] if head_commit else "" - # only set/update webhook on default branch pushes when the push actually - # touched the config file (config_changed above); this avoids spurious - # permission errors when the config merely aged out of the cache - # we also give maintainers the option to force-set the webhook: if the - # commit message contains [hubcast config], we'll set the webhook - if is_default_branch and (config_changed or "[hubcast config]" in commit_msg): - # setup callback webhook on GitLab - try: - await gl.set_webhook( - dest_org=repo_config.dest_org, - dest_repo=repo_config.dest_name, - gh_owner=src_owner, - gh_repo=src_repo_name, - gh_check=repo_config.check_name, - check_types=repo_config.check_types, - ) - except WebhookPermissionError as exc: - # the user is not a maintainer and we need to tell them to push config changes with higher permissions - exc.log(log) - await gh.set_check_status( - want_sha, - ERROR_CHECK_NAME, - "failure", - title=WEBHOOK_PERMISSION_DENIED_TITLE, - summary=WEBHOOK_PERMISSION_DENIED_SUMMARY, - ) - return - except HubcastError as exc: - # log for the hubcast admin and tell the user it's not their fault - exc.log(log) - await gh.set_check_status( - want_sha, - ERROR_CHECK_NAME, - "failure", - title=INTERNAL_ERROR_TITLE, - summary=INTERNAL_ERROR_SUMMARY, - ) - return - else: - log.info("Updated GitLab webhook", extra={"dest_fullname": dest_fullname}) +async def _sync_ref( + gh: GitHubClient, + gl: GitLabClient, + gl_user: str, + dest_remote_url: str, + sync_ref: str, + want_sha: str, + src_repo_url: str, + # auth rules differ by caller, which needs to provide its own closure + get_src_creds: Callable[[], Awaitable[dict[str, str]]], + check_name: str, + entity: str, +) -> bool: + """Sync `sync_ref` on the destination to `want_sha`, fetching from `src_repo_url`. + Permission issues and Repligit errors are reported as GitHub checks to `want_sha`. - # sync commits from GitHub -> GitLab + Returns True in a success state (up-to-date or sync performed), otherwise False. + """ gl_token = await gl.auth.authenticate_user(gl_user) try: @@ -170,41 +119,33 @@ async def sync_branch( log.info(PERMISSION_DENIED_SYNC_LOG_MSG) await gh.set_check_status( want_sha, - repo_config.check_name, + check_name, "failure", title=PERMISSION_DENIED_TITLE, summary=PERMISSION_DENIED_SUMMARY, ) - return + return False have_shas = set(gl_refs.values()) - from_sha = gl_refs.get(sync_ref) or ("0" * 40) - + from_sha = gl_refs.get(sync_ref) or NULL_SHA update_log_context(from_sha=from_sha, want_sha=want_sha) # directly check from_sha equals want_sha for cases where the sha has # already been mirrored but the ref is out-of-date. This is commonly the # case for tags that are created against an existing commit on a branch. if from_sha == want_sha: - log.info("Skipped branch sync - already up-to-date") - return - - log.info("Syncing branch") + log.info(f"Skipped {entity} sync - already up-to-date") + return True - gh_token = await gh.auth.authenticate_installation(gh.repo_owner, gh.repo_name) - - packfile = await fetch_pack( - src_repo_url, - want_sha, - have_shas, - username=gh.requester, # the username doesn't matter, but can't be empty - password=gh_token, - ) + # each caller has different rules for fetching the packfile from src_repo_url + src_creds = await get_src_creds() + packfile = await fetch_pack(src_repo_url, want_sha, have_shas, **src_creds) if packfile is None: raise HubcastError( f"Failed to fetch packfile for {want_sha} from {src_repo_url}" ) + log.info(f"Syncing {entity}") try: await send_pack( dest_remote_url, @@ -221,46 +162,38 @@ async def sync_branch( log.info(PERMISSION_DENIED_SYNC_LOG_MSG) await gh.set_check_status( want_sha, - repo_config.check_name, + check_name, "failure", title=PERMISSION_DENIED_TITLE, summary=PERMISSION_DENIED_SUMMARY, ) - return + return False # repligit except RefUpdateRejected as exc: hook_declined = str(exc) == HOOK_DECLINED_MSG await gh.set_check_status( want_sha, - repo_config.check_name, + check_name, "failure", title=HOOK_DECLINED_TITLE if hook_declined else INTERNAL_ERROR_TITLE, summary=HOOK_DECLINED_SUMMARY if hook_declined else INTERNAL_ERROR_SUMMARY, ) if not hook_declined: raise - return + return False - log.info("Synced branch") + log.info(f"Synced {entity}") + return True -@router.register("push", deleted=True) -async def remove_branch( - event: sansio.Event, - gh: GitHubClient, +async def _delete_ref( gl: GitLabClient, gl_user: str, - *arg, - **kwargs, + dest_remote_url: str, + sync_ref: str, + entity: str, ) -> None: - src_fullname = event.data["repository"]["full_name"] - sync_ref = event.data["ref"] - - repo_config = await get_repo_config(gh, src_fullname) - - dest_fullname = repo_config.dest_fullname - dest_remote_url = f"{gl.instance_url}/{dest_fullname}.git" - + """Delete `sync_ref` from the destination, if it exists.""" gl_token = await gl.auth.authenticate_user(gl_user) try: @@ -273,23 +206,20 @@ async def remove_branch( return head_sha = gl_refs.get(sync_ref) - - update_log_context(ref=sync_ref, head_sha=head_sha) + update_log_context(head_sha=head_sha) if head_sha is None: - log.info("Skipped branch removal - ref not found") + log.info(f"Skipped {entity} removal - ref not found") return - null_sha = "0" * 40 - - log.info("Deleting branch") + log.info(f"Deleting {entity}") try: await send_pack( dest_remote_url, sync_ref, head_sha, - null_sha, + NULL_SHA, b"", username=gl_user, password=gl_token, @@ -307,7 +237,137 @@ async def remove_branch( log.info(str(exc)) return - log.info("Deleted branch") + log.info(f"Deleted {entity}") + + +# ----------------------------------- +# Push Events +# ----------------------------------- +@router.register("push", deleted=False) +async def sync_branch( + event: sansio.Event, + gh: GitHubClient, + gl: GitLabClient, + gl_user: str, + *arg, + **kwargs, +) -> None: + """Sync the git branch referenced to GitLab.""" + src_repo_url = event.data["repository"]["clone_url"] + src_fullname = event.data["repository"]["full_name"] + src_owner, src_repo_name = src_fullname.split("/") + # the commit the push event is referencing + want_sha = event.data["after"] + sync_ref = event.data["ref"] + + update_log_context(ref=sync_ref) + + # skip branches from push events that are also pull requests + if await gh.get_prs(branch=sync_ref): + log.info("Skipped branch sync - branch has open PR") + return + + # only refresh config when a default branch push touches .github/hubcast.yml + default_branch = event.data["repository"]["default_branch"] + is_default_branch = sync_ref == f"refs/heads/{default_branch}" + config_changed = gh.repo_config_path in changed_files_from_push(event.data) + try: + repo_config = await get_repo_config( + gh, src_fullname, refresh=is_default_branch and config_changed + ) + except RepoConfigError as exc: + await report_config_error(gh, want_sha, exc) + return + + dest_fullname = repo_config.dest_fullname + dest_remote_url = f"{gl.instance_url}/{dest_fullname}.git" + head_commit = event.data.get("head_commit") + commit_msg = head_commit["message"] if head_commit else "" + + # only set/update webhook on default branch pushes when the push actually + # touched the config file (config_changed above); this avoids spurious + # permission errors when the config merely aged out of the cache + # we also give maintainers the option to force-set the webhook: if the + # commit message contains [hubcast config], we'll set the webhook + if is_default_branch and (config_changed or "[hubcast config]" in commit_msg): + # setup callback webhook on GitLab + try: + await gl.set_webhook( + dest_org=repo_config.dest_org, + dest_repo=repo_config.dest_name, + gh_owner=src_owner, + gh_repo=src_repo_name, + gh_check=repo_config.check_name, + check_types=repo_config.check_types, + ) + except WebhookPermissionError as exc: + # the user is not a maintainer and we need to tell them to push config changes with higher permissions + exc.log(log) + await gh.set_check_status( + want_sha, + ERROR_CHECK_NAME, + "failure", + title=WEBHOOK_PERMISSION_DENIED_TITLE, + summary=WEBHOOK_PERMISSION_DENIED_SUMMARY, + ) + return + except HubcastError as exc: + # log for the hubcast admin and tell the user it's not their fault + exc.log(log) + await gh.set_check_status( + want_sha, + ERROR_CHECK_NAME, + "failure", + title=INTERNAL_ERROR_TITLE, + summary=INTERNAL_ERROR_SUMMARY, + ) + return + else: + log.info("Updated GitLab webhook", extra={"dest_fullname": dest_fullname}) + + async def get_src_creds() -> dict[str, str]: + # push events can only come via the source repo, so we assume that the GitHub app can authenticate with its credentials + return { + "username": gh.requester, # the username doesn't matter, but can't be empty + "password": await gh.auth.authenticate_installation( + gh.repo_owner, gh.repo_name + ), + } + + await _sync_ref( + gh, + gl, + gl_user, + dest_remote_url, + sync_ref, + want_sha, + src_repo_url, + get_src_creds, + check_name=repo_config.check_name, + entity="branch", + ) + + +@router.register("push", deleted=True) +async def remove_branch( + event: sansio.Event, + gh: GitHubClient, + gl: GitLabClient, + gl_user: str, + *arg, + **kwargs, +) -> None: + src_fullname = event.data["repository"]["full_name"] + sync_ref = event.data["ref"] + + update_log_context(ref=sync_ref) + + repo_config = await get_repo_config(gh, src_fullname) + + dest_fullname = repo_config.dest_fullname + dest_remote_url = f"{gl.instance_url}/{dest_fullname}.git" + + await _delete_ref(gl, gl_user, dest_remote_url, sync_ref, entity="branch") # ----------------------------------- @@ -328,21 +388,12 @@ async def sync_pr( This isn't technically an event handler, but is used a couple different ways in this file. """ - pull_request_id = pull_request["number"] - src_repo_url = pull_request["head"]["repo"]["clone_url"] src_fullname = pull_request["head"]["repo"]["full_name"] base_fullname = pull_request["base"]["repo"]["full_name"] - - # pull requests coming from forks are pushed as branches in the form of - # pr- instead of as their branch name as conflicts could occur - # between multiple repositories is_pull_request_fork = src_fullname != base_fullname - if is_pull_request_fork: - sync_branch = f"pr-{pull_request_id}" - else: - sync_branch = pull_request["head"]["ref"] + sync_branch = _pr_sync_branch(pull_request) sync_ref = f"refs/heads/{sync_branch}" update_log_context(ref=sync_ref) @@ -375,99 +426,38 @@ async def sync_pr( dest_fullname = repo_config.dest_fullname dest_remote_url = f"{gl.instance_url}/{dest_fullname}.git" - gl_token = await gl.auth.authenticate_user(gl_user) - - try: - gl_refs = await ls_remote(dest_remote_url, username=gl_user, password=gl_token) - except ClientResponseError as exc: - if exc.status not in PERMISSION_DENIED_STATUSES: - raise - log.info(PERMISSION_DENIED_SYNC_LOG_MSG) - await gh.set_check_status( - want_sha, - repo_config.check_name, - "failure", - title=PERMISSION_DENIED_TITLE, - summary=PERMISSION_DENIED_SUMMARY, - ) - return - have_shas = set(gl_refs.values()) - from_sha = gl_refs.get(sync_ref) or ("0" * 40) - update_log_context(from_sha=from_sha, want_sha=want_sha) - - # directly check from_sha equals want_sha for cases where the sha has - # already been mirrored but the ref is out-of-date. This is commonly the - # case for tags that are created against an existing commit on a branch. - if from_sha == want_sha: - log.info("Skipped PR sync - already up-to-date") - else: # needs sync + async def get_src_creds() -> dict[str, str]: + # we should not try to authenticate if the source is a public fork, as our GitHub app credentials will not work + # in addition to the fact that they are public if is_pull_request_fork and not src_repo_private: - # no auth needed for public forks - src_creds = {} - else: - # authenticate if the PR comes from the src repository - src_creds = { - "username": gh.requester, # the username doesn't matter, but can't be empty - "password": await gh.auth.authenticate_installation( - gh.repo_owner, gh.repo_name - ), - } - - # fetch differential packfile with all new commits - packfile = await fetch_pack( - src_repo_url, - want_sha, - have_shas, - **src_creds, - ) - if packfile is None: - raise HubcastError( - f"Failed to fetch packfile for {want_sha} from {src_repo_url}" - ) - - # upload packfile to gitlab repository - log.info("Syncing PR") - try: - await send_pack( - dest_remote_url, - sync_ref, - from_sha, - want_sha, - packfile, - username=gl_user, - password=gl_token, - ) - except ClientResponseError as exc: - if exc.status not in PERMISSION_DENIED_STATUSES: - raise - log.info(PERMISSION_DENIED_SYNC_LOG_MSG) - await gh.set_check_status( - want_sha, - repo_config.check_name, - "failure", - title=PERMISSION_DENIED_TITLE, - summary=PERMISSION_DENIED_SUMMARY, - ) - return - # repligit - except RefUpdateRejected as exc: - hook_declined = str(exc) == HOOK_DECLINED_MSG - await gh.set_check_status( - want_sha, - repo_config.check_name, - "failure", - title=HOOK_DECLINED_TITLE if hook_declined else INTERNAL_ERROR_TITLE, - summary=HOOK_DECLINED_SUMMARY - if hook_declined - else INTERNAL_ERROR_SUMMARY, - ) - if not hook_declined: - raise - return + return {} + # use GH app credentials if the PR comes from the src repo + return { + "username": gh.requester, # the username doesn't matter, but can't be empty + "password": await gh.auth.authenticate_installation( + gh.repo_owner, gh.repo_name + ), + } + + synced = await _sync_ref( + gh, + gl, + gl_user, + dest_remote_url, + sync_ref, + want_sha, + src_repo_url, + get_src_creds, + check_name=repo_config.check_name, + entity="PR", + ) - log.info("Synced PR") + # sync failed (logged in _sync_ref) + if not synced: + return + # create MRs if configured # skip already created MRs if repo_config.create_mr and not await gl.get_mr( dest_fullname, sync_branch, default_branch @@ -525,7 +515,6 @@ async def remove_pr( **kwargs, ) -> None: pull_request = event.data["pull_request"] - pull_request_id = pull_request["number"] src_fullname = pull_request["head"]["repo"]["full_name"] base_fullname = pull_request["base"]["repo"]["full_name"] @@ -541,58 +530,17 @@ async def remove_pr( # pull request comes from an internal branch we should wait # to clean up the branch when the branch is deleted from the # internal repository - is_pull_request_fork = src_fullname != base_fullname - if not is_pull_request_fork: + if src_fullname == base_fullname: log.info("Skipped PR branch removal - internal branch") return - sync_ref = f"refs/heads/pr-{pull_request_id}" + sync_ref = f"refs/heads/{_pr_sync_branch(pull_request)}" update_log_context(ref=sync_ref) dest_fullname = repo_config.dest_fullname dest_remote_url = f"{gl.instance_url}/{dest_fullname}.git" - gl_token = await gl.auth.authenticate_user(gl_user) - try: - gl_refs = await ls_remote(dest_remote_url, username=gl_user, password=gl_token) - except ClientResponseError as exc: - if exc.status not in PERMISSION_DENIED_STATUSES: - raise - # we cannot set GitHub status checks for deleted refs, and we have no way to notify the user of this failure - log.info(PERMISSION_DENIED_DELETE_LOG_MSG) - return - - head_sha = gl_refs.get(sync_ref) - if head_sha is None: - log.info("Skipped PR branch removal - ref not found") - return - - null_sha = "0" * 40 - - log.info("Deleting PR branch") - try: - await send_pack( - dest_remote_url, - sync_ref, - head_sha, - null_sha, - b"", - username=gl_user, - password=gl_token, - ) - except ClientResponseError as exc: - if exc.status not in PERMISSION_DENIED_STATUSES: - raise - log.info(PERMISSION_DENIED_DELETE_LOG_MSG) - return - # repligit - except RefUpdateRejected as exc: - if str(exc) != HOOK_DECLINED_MSG: - raise - log.info(str(exc)) - return - - log.info("Deleted PR branch") + await _delete_ref(gl, gl_user, dest_remote_url, sync_ref, entity="PR branch") @router.register("issue_comment", action="created") @@ -679,17 +627,8 @@ async def respond_comment( pull_request = await gh.get_pr(pr_number) # get the branch this PR belongs to - src_fullname = pull_request["head"]["repo"]["full_name"] base_fullname = pull_request["base"]["repo"]["full_name"] - - # pull requests coming from forks are pushed as branches in the form of - # pr- instead of as their branch name as conflicts could occur - # between multiple repositories - is_pull_request_fork = src_fullname != base_fullname - if is_pull_request_fork: - branch = f"pr-{pr_number}" - else: - branch = pull_request["head"]["ref"] + branch = _pr_sync_branch(pull_request) update_log_context(branch=branch) @@ -701,11 +640,7 @@ async def respond_comment( pipeline_url = await gl.run_pipeline(dest_fullname, branch) except BadRequest as exc: if exc.status_code in PERMISSION_DENIED_STATUSES: - response = ( - DEACTIVATED_ACCOUNT_MSG - if DEACTIVATED_ACCOUNT_MARKER in str(exc) - else PERMISSION_DENIED_SUMMARY - ) + response = _permission_denied_response(exc) log.info("Pipeline failed to start - insufficient permissions") elif exc.status_code == 400: # \n to avoid indent markdown issues @@ -728,16 +663,8 @@ async def respond_comment( pull_request = await gh.get_pr(pr_number) # get the branch this PR belongs to - src_fullname = pull_request["head"]["repo"]["full_name"] base_fullname = pull_request["base"]["repo"]["full_name"] - # pull requests coming from forks are pushed as branches in the form of - # pr- instead of as their branch name as conflicts could occur - # between multiple repositories - is_pull_request_fork = src_fullname != base_fullname - if is_pull_request_fork: - branch = f"pr-{pr_number}" - else: - branch = pull_request["head"]["ref"] + branch = _pr_sync_branch(pull_request) update_log_context(branch=branch) @@ -750,11 +677,7 @@ async def respond_comment( except BadRequest as exc: if exc.status_code not in PERMISSION_DENIED_STATUSES: raise - response = ( - DEACTIVATED_ACCOUNT_MSG - if DEACTIVATED_ACCOUNT_MARKER in str(exc) - else PERMISSION_DENIED_SUMMARY - ) + response = _permission_denied_response(exc) log.info("Pipeline ID fetch failed - insufficient permissions") else: if pipeline_id: @@ -765,11 +688,7 @@ async def respond_comment( except BadRequest as exc: if exc.status_code not in PERMISSION_DENIED_STATUSES: raise - response = ( - DEACTIVATED_ACCOUNT_MSG - if DEACTIVATED_ACCOUNT_MARKER in str(exc) - else PERMISSION_DENIED_SUMMARY - ) + response = _permission_denied_response(exc) log.info("Jobs restart failed - insufficient permissions") else: response = f"I've retried any failed jobs in the [pipeline]({pipeline_url})!" @@ -795,6 +714,18 @@ async def respond_comment( PIPELINE_DETAILS_URL_RE = re.compile(r"/-/pipelines/(\d+)/?$") +def _is_deactivated_account(exc: BadRequest) -> bool: + """Whether a GitLab permission-denied error was caused by a deactivated account.""" + return DEACTIVATED_ACCOUNT_MARKER in str(exc) + + +def _permission_denied_response(exc: BadRequest) -> str: + """User-facing message for a GitLab permission-denied error from a comment command.""" + if _is_deactivated_account(exc): + return DEACTIVATED_ACCOUNT_MSG + return PERMISSION_DENIED_SUMMARY + + async def _fail_check_from_pipeline_error( gh: GitHubClient, check_name: str, @@ -803,7 +734,7 @@ async def _fail_check_from_pipeline_error( ) -> None: """Set a failing check status from a GitLab pipeline/job start error.""" if exc.status_code in PERMISSION_DENIED_STATUSES: - deactivated = DEACTIVATED_ACCOUNT_MARKER in str(exc) + deactivated = _is_deactivated_account(exc) message = DEACTIVATED_ACCOUNT_MSG if deactivated else PERMISSION_DENIED_TITLE summary = "" if deactivated else PERMISSION_DENIED_SUMMARY elif exc.status_code == 400: From 58a874932848b95efdc7e7cc9e25c989d8c94e86 Mon Sep 17 00:00:00 2001 From: Caetano Melone Date: Wed, 19 Aug 2026 19:16:50 -0700 Subject: [PATCH 2/3] address review Signed-off-by: Caetano Melone --- src/hubcast/web/github/routes.py | 36 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/hubcast/web/github/routes.py b/src/hubcast/web/github/routes.py index 4966cfb..f97c719 100644 --- a/src/hubcast/web/github/routes.py +++ b/src/hubcast/web/github/routes.py @@ -77,7 +77,7 @@ async def report_config_error(gh: GitHubClient, sha: str, exc: RepoConfigError) ) -def _pr_sync_branch(pull_request: dict[str, Any]) -> str: +def _pr_branch_name(pull_request: dict[str, Any]) -> str: """Return the branch name used on the destination for this PR. Pull requests coming from forks are pushed as branches in the form of @@ -91,6 +91,18 @@ def _pr_sync_branch(pull_request: dict[str, Any]) -> str: return pull_request["head"]["ref"] +def _is_deactivated_account(exc: BadRequest) -> bool: + """Whether a GitLab permission-denied error was caused by a deactivated account.""" + return DEACTIVATED_ACCOUNT_MARKER in str(exc) + + +def _permission_denied_response(exc: BadRequest) -> str: + """User-facing message for a GitLab permission-denied error from a comment command.""" + if _is_deactivated_account(exc): + return DEACTIVATED_ACCOUNT_MSG + return PERMISSION_DENIED_SUMMARY + + async def _sync_ref( gh: GitHubClient, gl: GitLabClient, @@ -393,7 +405,7 @@ async def sync_pr( base_fullname = pull_request["base"]["repo"]["full_name"] is_pull_request_fork = src_fullname != base_fullname - sync_branch = _pr_sync_branch(pull_request) + sync_branch = _pr_branch_name(pull_request) sync_ref = f"refs/heads/{sync_branch}" update_log_context(ref=sync_ref) @@ -530,11 +542,11 @@ async def remove_pr( # pull request comes from an internal branch we should wait # to clean up the branch when the branch is deleted from the # internal repository - if src_fullname == base_fullname: + if not src_fullname != base_fullname: log.info("Skipped PR branch removal - internal branch") return - sync_ref = f"refs/heads/{_pr_sync_branch(pull_request)}" + sync_ref = f"refs/heads/{_pr_branch_name(pull_request)}" update_log_context(ref=sync_ref) dest_fullname = repo_config.dest_fullname @@ -628,7 +640,7 @@ async def respond_comment( # get the branch this PR belongs to base_fullname = pull_request["base"]["repo"]["full_name"] - branch = _pr_sync_branch(pull_request) + branch = _pr_branch_name(pull_request) update_log_context(branch=branch) @@ -664,7 +676,7 @@ async def respond_comment( # get the branch this PR belongs to base_fullname = pull_request["base"]["repo"]["full_name"] - branch = _pr_sync_branch(pull_request) + branch = _pr_branch_name(pull_request) update_log_context(branch=branch) @@ -714,18 +726,6 @@ async def respond_comment( PIPELINE_DETAILS_URL_RE = re.compile(r"/-/pipelines/(\d+)/?$") -def _is_deactivated_account(exc: BadRequest) -> bool: - """Whether a GitLab permission-denied error was caused by a deactivated account.""" - return DEACTIVATED_ACCOUNT_MARKER in str(exc) - - -def _permission_denied_response(exc: BadRequest) -> str: - """User-facing message for a GitLab permission-denied error from a comment command.""" - if _is_deactivated_account(exc): - return DEACTIVATED_ACCOUNT_MSG - return PERMISSION_DENIED_SUMMARY - - async def _fail_check_from_pipeline_error( gh: GitHubClient, check_name: str, From 975b1458d48c5455f5555c63148c51489ce46543 Mon Sep 17 00:00:00 2001 From: Caetano Melone Date: Thu, 20 Aug 2026 08:38:44 -0700 Subject: [PATCH 3/3] fix ruff Signed-off-by: Caetano Melone --- src/hubcast/web/github/routes.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hubcast/web/github/routes.py b/src/hubcast/web/github/routes.py index f97c719..f0af2b9 100644 --- a/src/hubcast/web/github/routes.py +++ b/src/hubcast/web/github/routes.py @@ -542,7 +542,8 @@ async def remove_pr( # pull request comes from an internal branch we should wait # to clean up the branch when the branch is deleted from the # internal repository - if not src_fullname != base_fullname: + is_pull_request_fork = src_fullname != base_fullname + if not is_pull_request_fork: log.info("Skipped PR branch removal - internal branch") return