From cc73429fb7bf95e94c546c7283c87c3baf1410b6 Mon Sep 17 00:00:00 2001 From: Caetano Melone Date: Tue, 25 Aug 2026 09:26:07 -0700 Subject: [PATCH] fix: don't react positively to approve comment if sync is skipped in [this PR](https://github.com/llnl/RAJA/pull/2080), someone called the approve command, but the sync was skipped because the repo had draft PR mirroring disabled. we erroneously reacted with a +1 to the approve comment this change checks if the PR was actually synced before reacting positively Signed-off-by: Caetano Melone --- src/hubcast/web/github/routes.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/hubcast/web/github/routes.py b/src/hubcast/web/github/routes.py index d2122aa..7327bf4 100644 --- a/src/hubcast/web/github/routes.py +++ b/src/hubcast/web/github/routes.py @@ -451,10 +451,12 @@ async def sync_pr( src_repo_private: bool, want_sha: str, default_branch: str, -) -> None: +) -> bool: """Sync the git fork/branch referenced in a PR to GitLab. This isn't technically an event handler, but is used a couple different ways in this file. + + Returns boolean values based on whether the sync occurred or was skipped. """ src_repo_url = pull_request["head"]["repo"]["clone_url"] src_fullname = pull_request["head"]["repo"]["full_name"] @@ -472,7 +474,7 @@ async def sync_pr( "Skipped PR sync - private fork", extra={"fork_fullname": pull_request["head"]["repo"]["full_name"]}, ) - return + return False changed_files = await gh.get_pr_files(pull_request["number"]) config_changed = gh.repo_config_path in changed_files @@ -485,7 +487,7 @@ async def sync_pr( if not config_changed: await report_config_error(gh, want_sha, exc) await validate_config_change(gh, changed_files, want_sha) - return + return False # validate the changes when the default branch config doesn't have issues await validate_config_change(gh, changed_files, want_sha) @@ -499,7 +501,7 @@ async def sync_pr( title="Hubcast disables sync for draft PRs.", ) log.info("Skipped PR sync - draft PR") - return + return False dest_fullname = repo_config.dest_fullname dest_remote_url = f"{gl.instance_url}/{dest_fullname}.git" @@ -532,7 +534,7 @@ async def get_src_creds() -> dict[str, str]: # sync failed (logged in _sync_ref) if not synced: - return + return False # create MRs if configured # skip already created MRs @@ -549,6 +551,8 @@ async def get_src_creds() -> dict[str, str]: ) log.info("Created MR", extra={"branch": sync_branch}) + return True + @router.register("pull_request", action="opened") @router.register("pull_request", action="reopened") @@ -670,7 +674,7 @@ async def respond_comment( pull_request = event.data["pull_request"] src_repo_private = pull_request["head"]["repo"]["private"] # sync the approved commit explicitly - await sync_pr( + synced = await sync_pr( pull_request, gh, gl, @@ -679,11 +683,12 @@ async def respond_comment( want_sha=commit_sha, default_branch=event.data["repository"]["default_branch"], ) - plus_one = True - log.info( - "Mirrored ref with approval from review comment", - extra={"ref": commit_sha}, - ) + if synced: + plus_one = True + log.info( + "Mirrored ref with approval from review comment", + extra={"ref": commit_sha}, + ) else: response = ( "To mirror this PR, please use the "