From f284e732078c29cd4ec9a5f9de035ac675e38386 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Thu, 12 Mar 2026 11:39:29 +0200 Subject: [PATCH 1/3] refactor: migrate cherry-pick PR creation from hub to gh CLI (#1023) Replace deprecated hub CLI (v2.14.2, archived) with official gh CLI for creating cherry-pick pull requests. Changes: - Dockerfile: replace hub installation with gh CLI v2.67.0 - runner_handler.py: replace `hub pull-request` with `gh pr create` - tests: update cherry-pick test assertions for gh CLI flags Signed-off-by: Meni Yakove --- Dockerfile | 5 ++--- .../libs/handlers/runner_handler.py | 20 ++++++++++++------- webhook_server/tests/test_runner_handler.py | 16 +++++++-------- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/Dockerfile b/Dockerfile index e8c839a8..26c5a9c2 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,6 +14,7 @@ RUN systemd-machine-id-setup RUN dnf --nodocs --setopt=install_weak_deps=False --disable-repo=fedora-cisco-openh264 -y install dnf-plugins-core \ && dnf --nodocs --setopt=install_weak_deps=False --disable-repo=fedora-cisco-openh264 -y update \ && dnf --nodocs --setopt=install_weak_deps=False --disable-repo=fedora-cisco-openh264 -y install \ + gh \ git \ unzip \ gcc \ @@ -71,9 +72,7 @@ RUN set -ex \ && curl --fail -vL https://mirror.openshift.com/pub/openshift-v4/clients/rosa/latest/rosa-linux.tar.gz | tar -C $BIN_DIR -xzvf - rosa \ && chmod +x $BIN_DIR/rosa \ && curl --fail -vL https://github.com/regclient/regclient/releases/latest/download/regctl-linux-amd64 -o $BIN_DIR/regctl \ - && chmod +x $BIN_DIR/regctl \ - && curl --fail -vL https://github.com/mislav/hub/releases/download/v2.14.2/hub-linux-amd64-2.14.2.tgz | tar --wildcards --strip-components=2 -C $BIN_DIR -xzvf - '*/bin/hub' \ - && chmod +x $BIN_DIR/hub + && chmod +x $BIN_DIR/regctl # Copy dependency manifests first for uv sync cache stability COPY --chown=$USERNAME:$USERNAME pyproject.toml uv.lock README.md $APP_DIR/ diff --git a/webhook_server/libs/handlers/runner_handler.py b/webhook_server/libs/handlers/runner_handler.py index b2c5a0c1..7a05bc3f 100644 --- a/webhook_server/libs/handlers/runner_handler.py +++ b/webhook_server/libs/handlers/runner_handler.py @@ -714,19 +714,25 @@ async def cherry_pick( async with self._checkout_worktree(pull_request=pull_request) as (success, worktree_path, out, err): git_cmd = f"git --work-tree={worktree_path} --git-dir={worktree_path}/.git" - hub_cmd = f"GITHUB_TOKEN={github_token} hub --work-tree={worktree_path} --git-dir={worktree_path}/.git" - assignee_flag = f" -a {shlex.quote(pr_author)}" if assign_to_pr_owner else "" + assignee_flag = f" --assignee {shlex.quote(pr_author)}" if assign_to_pr_owner else "" + pr_title = f"{CHERRY_PICKED_LABEL}: [{target_branch}] {commit_msg_striped}" + pr_body = ( + f"Cherry-pick from `{source_branch}` branch, original PR: {pull_request_url}, PR owner: {pr_author}" + ) + repo_full_name = self.github_webhook.repository_full_name commands: list[str] = [ f"{git_cmd} checkout {target_branch}", f"{git_cmd} pull origin {target_branch}", f"{git_cmd} checkout -b {new_branch_name} origin/{target_branch}", f"{git_cmd} cherry-pick {commit_hash}", f"{git_cmd} push origin {new_branch_name}", - f'bash -c "{hub_cmd} pull-request -b {target_branch} ' - f"-h {new_branch_name} -l {CHERRY_PICKED_LABEL} {assignee_flag} " - f"-m '{CHERRY_PICKED_LABEL}: [{target_branch}] " - f"{commit_msg_striped}' -m 'Cherry-pick from `{source_branch}` branch, " - f"original PR: {pull_request_url}, PR owner: {pr_author}'\"", + f"GH_TOKEN={github_token} gh pr create --repo {shlex.quote(repo_full_name)}" + f" --base {shlex.quote(target_branch)}" + f" --head {shlex.quote(new_branch_name)}" + f" --label {shlex.quote(CHERRY_PICKED_LABEL)}" + f"{assignee_flag}" + f" --title {shlex.quote(pr_title)}" + f" --body {shlex.quote(pr_body)}", ] output: CheckRunOutput = { diff --git a/webhook_server/tests/test_runner_handler.py b/webhook_server/tests/test_runner_handler.py index 3b0bfbd5..059284f2 100644 --- a/webhook_server/tests/test_runner_handler.py +++ b/webhook_server/tests/test_runner_handler.py @@ -1263,8 +1263,9 @@ async def test_cherry_pick_assigns_pr_author(self, runner_handler: RunnerHandler mocks.comment.assert_called_once() assert mocks.to_thread.call_count == 3 last_cmd = mocks.run_cmd.call_args_list[-1] - hub_command = last_cmd.kwargs.get("command", last_cmd.args[0] if last_cmd.args else "") - assert "-a 'test-pr-author'" in hub_command or "-a test-pr-author" in hub_command + gh_command = last_cmd.kwargs.get("command", last_cmd.args[0] if last_cmd.args else "") + assert "--assignee" in gh_command + assert "test-pr-author" in gh_command @pytest.mark.asyncio async def test_cherry_pick_requested_by_uses_pr_owner( @@ -1277,12 +1278,11 @@ async def test_cherry_pick_requested_by_uses_pr_owner( mocks.set_success.assert_called_once() mocks.comment.assert_called_once() last_cmd = mocks.run_cmd.call_args_list[-1] - hub_command = last_cmd.kwargs.get("command", last_cmd.args[0] if last_cmd.args else "") - expected_msg = ( - f"Cherry-pick from `main` branch, original PR: {mock_pull_request.html_url}, PR owner: test-pr-author" - ) - assert expected_msg in hub_command - assert "-a 'test-pr-author'" in hub_command or "-a test-pr-author" in hub_command + gh_command = last_cmd.kwargs.get("command", last_cmd.args[0] if last_cmd.args else "") + assert "Cherry-pick from" in gh_command + assert mock_pull_request.html_url in gh_command + assert "test-pr-author" in gh_command + assert "--assignee" in gh_command assert mocks.to_thread.call_count == 3 @pytest.mark.asyncio From beb4924a6f680ff6c70248dcb5153f7ccb344987 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Thu, 12 Mar 2026 12:02:20 +0200 Subject: [PATCH 2/3] fix: pass GH_TOKEN via subprocess env instead of command prefix The `run_command()` function uses `asyncio.create_subprocess_exec` (no shell), so `GH_TOKEN=... gh pr create` treated `GH_TOKEN=...` as the executable name. Fix by passing the token via the `env` kwarg to `run_command()`, which forwards it to `create_subprocess_exec`. Each subprocess gets its own isolated env copy, so parallel execution with different tokens per repo is safe. Signed-off-by: Meni Yakove --- .../libs/handlers/runner_handler.py | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/webhook_server/libs/handlers/runner_handler.py b/webhook_server/libs/handlers/runner_handler.py index 7a05bc3f..6bf10d96 100644 --- a/webhook_server/libs/handlers/runner_handler.py +++ b/webhook_server/libs/handlers/runner_handler.py @@ -1,5 +1,6 @@ import asyncio import contextlib +import os import re import shlex import shutil @@ -720,20 +721,22 @@ async def cherry_pick( f"Cherry-pick from `{source_branch}` branch, original PR: {pull_request_url}, PR owner: {pr_author}" ) repo_full_name = self.github_webhook.repository_full_name - commands: list[str] = [ + git_commands: list[str] = [ f"{git_cmd} checkout {target_branch}", f"{git_cmd} pull origin {target_branch}", f"{git_cmd} checkout -b {new_branch_name} origin/{target_branch}", f"{git_cmd} cherry-pick {commit_hash}", f"{git_cmd} push origin {new_branch_name}", - f"GH_TOKEN={github_token} gh pr create --repo {shlex.quote(repo_full_name)}" + ] + gh_pr_command = ( + f"gh pr create --repo {shlex.quote(repo_full_name)}" f" --base {shlex.quote(target_branch)}" f" --head {shlex.quote(new_branch_name)}" f" --label {shlex.quote(CHERRY_PICKED_LABEL)}" f"{assignee_flag}" f" --title {shlex.quote(pr_title)}" - f" --body {shlex.quote(pr_body)}", - ] + f" --body {shlex.quote(pr_body)}" + ) output: CheckRunOutput = { "title": "Cherry-pick details", @@ -744,7 +747,7 @@ async def cherry_pick( output["text"] = self.check_run_handler.get_check_run_text(out=out, err=err) await self.check_run_handler.set_check_failure(name=CHERRY_PICKED_LABEL, output=output) - for cmd in commands: + for cmd in git_commands: rc, out, err = await run_command( command=cmd, log_prefix=self.log_prefix, @@ -782,6 +785,33 @@ async def cherry_pick( ) return + # Run gh pr create with GH_TOKEN passed via env (not command prefix) + # Each subprocess gets its own env copy, safe for parallel execution + rc, out, err = await run_command( + command=gh_pr_command, + log_prefix=self.log_prefix, + redact_secrets=[github_token], + mask_sensitive=self.github_webhook.mask_sensitive, + env={**os.environ, "GH_TOKEN": github_token}, + ) + if not rc: + output["text"] = self.check_run_handler.get_check_run_text(err=err, out=out) + await self.check_run_handler.set_check_failure(name=CHERRY_PICKED_LABEL, output=output) + redacted_out = _redact_secrets( + out, + [github_token], + mask_sensitive=self.github_webhook.mask_sensitive, + ) + redacted_err = _redact_secrets( + err, + [github_token], + mask_sensitive=self.github_webhook.mask_sensitive, + ) + self.logger.error( + f"{self.log_prefix} Cherry pick PR creation failed: {redacted_out} --- {redacted_err}" + ) + return + output["text"] = self.check_run_handler.get_check_run_text(err=err, out=out) await self.check_run_handler.set_check_success(name=CHERRY_PICKED_LABEL, output=output) From 1fce32263ed5f18e5af50ee67661b82a9662f521 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Thu, 12 Mar 2026 12:16:11 +0200 Subject: [PATCH 3/3] fix: add early return on worktree failure and actionable comment on PR creation failure - Add `return` after worktree setup failure to prevent fall-through into git commands loop (pre-existing bug) - Add PR comment with manual recovery instructions when `gh pr create` fails but branch was already pushed Signed-off-by: Meni Yakove --- webhook_server/libs/handlers/runner_handler.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/webhook_server/libs/handlers/runner_handler.py b/webhook_server/libs/handlers/runner_handler.py index 6bf10d96..4923050a 100644 --- a/webhook_server/libs/handlers/runner_handler.py +++ b/webhook_server/libs/handlers/runner_handler.py @@ -746,6 +746,7 @@ async def cherry_pick( if not success: output["text"] = self.check_run_handler.get_check_run_text(out=out, err=err) await self.check_run_handler.set_check_failure(name=CHERRY_PICKED_LABEL, output=output) + return for cmd in git_commands: rc, out, err = await run_command( @@ -797,6 +798,20 @@ async def cherry_pick( if not rc: output["text"] = self.check_run_handler.get_check_run_text(err=err, out=out) await self.check_run_handler.set_check_failure(name=CHERRY_PICKED_LABEL, output=output) + await asyncio.to_thread( + pull_request.create_issue_comment, + f"**Cherry-pick branch created, but PR creation failed**\n" + f"Branch `{new_branch_name}` was pushed to the repository.\n" + f"Create the PR manually:\n" + "```\n" + f"gh pr create --repo {repo_full_name}" + f" --base {target_branch}" + f" --head {new_branch_name}" + f" --label {CHERRY_PICKED_LABEL}" + f" --title '{pr_title}'" + f" --body '{pr_body}'\n" + "```", + ) redacted_out = _redact_secrets( out, [github_token],