Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ pr-split merge --auto

`--auto` is not fire-and-forget: after queueing a batch, `merge` waits for every PR in it to reach `MERGED` before moving on to the dependent batch, polling GitHub every 10 seconds for up to 10 minutes per batch. If a PR is still unmerged when the timeout expires, the command stops before the dependent batch and exits 1 (webhook `exit_reason: incomplete_batch`). A PR closed while waiting is listed as `<id> (CLOSED)` among the skipped PRs, its dependants are skipped as blocked, and the run exits 1 (webhook `exit_reason: pr_closed`). Either way, re-run `pr-split merge --auto` once CI has caught up to continue from where it left off.

Use `--notify` to POST merge results to a webhook URL (e.g. Slack, Discord):
Use `--notify` to POST merge results to an http(s) webhook URL (e.g. Slack, Discord); other schemes are rejected up front, and a delivery failure (timeout, non-2xx) is logged as a warning without changing the exit code:

```bash
pr-split merge --notify https://hooks.slack.com/...
Expand Down
19 changes: 18 additions & 1 deletion pr_split/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import shutil
import tempfile
import time
import urllib.parse
import urllib.request
from collections.abc import Callable, Generator
from concurrent.futures import ThreadPoolExecutor, as_completed
Expand Down Expand Up @@ -2070,6 +2071,21 @@ def _poll_for_merged(
return actually_merged


def _validate_webhook_url(value: str | None) -> str | None:
"""Accept only http(s) webhook URLs.

urllib happily opens file:// (and reports "Webhook notification sent"),
and a bare hostname fails only after the merges with an obscure
"unknown url type" warning.
"""
if value is None:
return None
parts = urllib.parse.urlsplit(value)
if parts.scheme not in ("http", "https") or not parts.netloc:
raise typer.BadParameter(f"must be an http(s) URL, got '{value}'")
return value


def _send_webhook(url: str, payload: dict[str, object]) -> None:
try:
data = json_mod.dumps(payload).encode("utf-8")
Expand Down Expand Up @@ -2100,8 +2116,9 @@ def merge_all(
str | None,
typer.Option(
"--notify",
help="Webhook URL to POST merge results to",
help="Webhook URL (http/https) to POST merge results to",
envvar="PR_SPLIT_WEBHOOK_URL",
callback=_validate_webhook_url,
),
] = None,
) -> None:
Expand Down
31 changes: 31 additions & 0 deletions tests/test_cli_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from __future__ import annotations

import re
import subprocess
from pathlib import Path
from unittest.mock import MagicMock, patch
Expand Down Expand Up @@ -1949,3 +1950,33 @@ def test_yes_skips_the_confirmation_prompt(
mock_confirm.assert_not_called()
mock_create.assert_called_once()
assert "stop here" in result.output


def _flat(output: str) -> str:
# Rich draws the error in a box that wraps text (and, on CI, colours it
# with ANSI codes); strip both and collapse whitespace so a phrase can be
# matched regardless of wrapping.
plain = re.sub(r"\x1b\[[0-9;]*m", "", output)
return " ".join(re.sub(r"[│╭╮╰╯─]", " ", plain).split())


class TestNotifyUrlValidation:
@pytest.mark.parametrize("bad", ["file:///tmp/wh.txt", "not-a-url", "ftp://x/y", "https://"])
def test_non_http_urls_are_rejected_before_anything_runs(self, bad: str) -> None:
with patch("pr_split.cli.plan_exists") as mock_pe:
result = runner.invoke(app, ["merge", "--notify", bad])
assert result.exit_code == 2
assert "must be an http(s) URL" in _flat(result.output)
mock_pe.assert_not_called()

def test_env_var_is_validated_too(self) -> None:
with patch("pr_split.cli.plan_exists") as mock_pe:
result = runner.invoke(app, ["merge"], env={"PR_SPLIT_WEBHOOK_URL": "file:///x"})
assert result.exit_code == 2
mock_pe.assert_not_called()

@patch("pr_split.cli.plan_exists", return_value=False)
def test_http_urls_pass(self, mock_pe: MagicMock) -> None:
result = runner.invoke(app, ["merge", "--notify", "https://hooks.example/abc"])
assert result.exit_code == 0
mock_pe.assert_called_once()