From 4ed855fb5d1b108733bf2b52dd281277482ab053 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Wed, 29 Apr 2026 14:09:34 +0200 Subject: [PATCH 1/2] fix(utils): decode CommandError stdout safely so __str__ never raises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CommandError.__str__` decoded captured stdout as strict UTF-8, so any non-UTF-8 bytes from a subprocess (legacy locales, binary diff fragments, truncated multi-byte sequences) would turn `str(error)` into a `UnicodeDecodeError`. Every error-formatting site is affected — the top-level CLI handler in `cli.py:104`, log messages, etc. — converting the failure into an unhandled traceback instead of a clean message. Switch to `decode(errors="replace")`. Invalid bytes become the U+FFFD replacement character; all other formatting is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) Change-Id: I3e3fd853218a6c4294b17e9529794320288124e5 --- mergify_cli/tests/test_utils.py | 7 +++++++ mergify_cli/utils.py | 8 +++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/mergify_cli/tests/test_utils.py b/mergify_cli/tests/test_utils.py index 2114ee5c..469ba475 100644 --- a/mergify_cli/tests/test_utils.py +++ b/mergify_cli/tests/test_utils.py @@ -31,6 +31,13 @@ import pathlib +def test_command_error_str_handles_non_utf8_stdout() -> None: + # Some git invocations (e.g. legacy locales) can emit non-UTF-8 bytes; + # str(CommandError) must not raise — error paths depend on it. + error = utils.CommandError(("git", "show", "abc"), 1, b"\xff\xfe broken") + assert "failed to run `git show abc`" in str(error) + + @pytest.mark.usefixtures("_git_repo") async def test_get_branch_name() -> None: assert await utils.git_get_branch_name() == "main" diff --git a/mergify_cli/utils.py b/mergify_cli/utils.py index 4f5addd1..3d875a1b 100644 --- a/mergify_cli/utils.py +++ b/mergify_cli/utils.py @@ -92,7 +92,13 @@ class CommandError(Exception): stdout: bytes def __str__(self) -> str: - return f"failed to run `{' '.join(self.command_args)}`: {self.stdout.decode()}" + # ``errors="replace"`` so str(CommandError) never raises on + # non-UTF-8 process output — callers in error paths (warnings, + # CLI top-level handler) rely on this being safe. + return ( + f"failed to run `{' '.join(self.command_args)}`: " + f"{self.stdout.decode(errors='replace')}" + ) class MergifyError(click.ClickException): From 525f374c57cac2e1088b333e85c6af60e7501ee5 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Wed, 29 Apr 2026 14:21:07 +0200 Subject: [PATCH 2/2] fix(stack): surface fetch_old_pr_heads failures instead of swallowing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The exception handler around `fetch_old_pr_heads` previously did a silent `pass`, leaving every revision-history entry stamped "unknown" with no clue why. When a real failure happens (fork remote without `refs/pull/N/head`, network error, missing permissions, …) the user has no signal that anything went wrong — they only see the symptom in the PR comment days later. Log the underlying error in orange so the cause is visible at push time. The exception text is run through `rich.markup.escape` so any `[`/`]` in the git error message is rendered literally rather than parsed as Rich markup. Behaviour stays non-fatal: classification still falls back to "unknown" and the push proceeds. Co-Authored-By: Claude Opus 4.7 (1M context) Change-Id: Id33459ea069be228f73fb02a7f813f16db145b71 --- mergify_cli/stack/push.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/mergify_cli/stack/push.py b/mergify_cli/stack/push.py index 75219675..033d29e8 100644 --- a/mergify_cli/stack/push.py +++ b/mergify_cli/stack/push.py @@ -24,6 +24,8 @@ import sys import typing +import rich.markup + from mergify_cli import console from mergify_cli import console_error from mergify_cli import utils @@ -582,8 +584,16 @@ async def stack_push( with console.status("Fetching old PR heads for comparison..."): try: await fetch_old_pr_heads(remote, updated_pr_numbers) - except utils.CommandError: - pass # Non-fatal: change type will be "unknown" + except utils.CommandError as exc: + # Non-fatal: change type will be "unknown" — but surface + # the underlying error so the user can fix it. Escape the + # exception text since it can contain `[`/`]` that Rich + # would otherwise interpret as markup tags. + console.log( + f"[orange]Could not fetch old PR heads; revision-history " + f"change types will fall back to 'unknown': " + f"{rich.markup.escape(str(exc))}[/]", + ) # Detect change types before force-push overwrites refs change_types: dict[str, str] = {}