commit be2a77ffdaef9cb58cd1a638c0bee140f7812211 equwal <13551856+equwal@users.noreply.github.com> 2026-09-26 21:19:02 -0700 Open pull requests from a fork to its upstream repository handoff.py pr --repo UPSTREAM_OWNER/NAME opened a form that compared a branch of the upstream repository, not the branch of the fork. GitHub needs OWNER:BRANCH for a branch in another repository. pr_request now adds the owner of the remote when --repo names another repository. compare_url keeps the ":" of OWNER:BRANCH. find_open_pr filters the pull requests by the owner of the branch, because gh filters by the branch name only. The skill, the README, and the rule tell Claude to give --repo for a pull request to the upstream project of a fork.
README.md | 2 ++ rules/deploy-handoff.md | 3 +- skills/deploy-handoff/SKILL.md | 1 + skills/deploy-handoff/handoff.py | 44 ++++++++++++++++++++----- skills/deploy-handoff/test_handoff.py | 62 +++++++++++++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 10 deletions(-)
diff --git a/README.md b/README.md index 112062a..f313862 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,8 @@ Open the pull request form for the current branch: python3 skills/deploy-handoff/handoff.py pr --title "Add CSV export" --body-file body.md ``` +For a pull request from a fork to its upstream repository, push the branch to the fork and add `--repo UPSTREAM_OWNER/NAME`. The form then compares the branch of the fork with the upstream repository. + The script prints one JSON object, for example `{"status": "done", "note": ""}`. [SKILL.md](skills/deploy-handoff/SKILL.md) lists all statuses and exit codes. ## Rule diff --git a/rules/deploy-handoff.md b/rules/deploy-handoff.md index b4fa83a..b260267 100644 --- a/rules/deploy-handoff.md +++ b/rules/deploy-handoff.md @@ -83,8 +83,9 @@ Open a pull request only when one of these is true: - The user asked for a pull request. - The push failed because the branch is protected, or the repository needs a review. - The repository belongs to a client, an employer, or another team. +- The repository is a fork, and the change is useful to the upstream project. -Open each pull request with `handoff.py pr` from the skill. Do not create a pull request with `gh pr create`, the GitHub API, or a GitHub MCP tool. +Open each pull request with `handoff.py pr` from the skill. For a pull request from a fork to its upstream repository, add `--repo UPSTREAM_OWNER/NAME`. Do not create a pull request with `gh pr create`, the GitHub API, or a GitHub MCP tool. Never use `--no-verify`. Never push when the test suite fails. diff --git a/skills/deploy-handoff/SKILL.md b/skills/deploy-handoff/SKILL.md index 602c9f0..bac242c 100644 --- a/skills/deploy-handoff/SKILL.md +++ b/skills/deploy-handoff/SKILL.md @@ -148,6 +148,7 @@ The script opens the GitHub form with the title and the description filled in. T - The head is the current branch. The base is the default branch on GitHub. Use `--head` and `--base` to change them. - The script refuses the branch if the remote does not have the local commit. - The script gets OWNER/NAME from the URL of `origin`. Use `--remote` or `--repo OWNER/NAME` to change it. +- For a pull request from a fork to its upstream repository, push the branch to the fork. Then give `--repo UPSTREAM_OWNER/NAME`. The script names the branch as OWNER:BRANCH, with OWNER from the URL of the remote. - Do not create a pull request with `gh pr create`, the GitHub API, or another tool. ## 4. Wait for the answer diff --git a/skills/deploy-handoff/handoff.py b/skills/deploy-handoff/handoff.py index 411f623..a4ccf13 100644 --- a/skills/deploy-handoff/handoff.py +++ b/skills/deploy-handoff/handoff.py @@ -341,10 +341,22 @@ def github_repo(remote_url: str) -> str: return match["repo"] +def fork_owner(remote_url: str, repo: str) -> str | None: + """Return the owner of a GitHub remote that is not repo, for example a fork of repo, or None.""" + match = GITHUB_REMOTE_RE.fullmatch(remote_url.strip()) + if match is None or match["repo"].lower() == repo.lower(): + return None + return match["repo"].split("/")[0] + + def compare_url(repo: str, base: str | None, head: str, title: str, body: str) -> str: - """Return the GitHub page that shows a new pull request form with title and body filled in.""" - # Without a base, GitHub compares head with the default branch. - refs = quote(head) if base is None else f"{quote(base)}...{quote(head)}" + """Return the GitHub page that shows a new pull request form with title and body filled in. + + head is BRANCH, or OWNER:BRANCH for a branch in a fork of repo. + """ + # Without a base, GitHub compares head with the default branch. A branch name has no ":". + head_ref = quote(head, safe="/:") + refs = head_ref if base is None else f"{quote(base)}...{head_ref}" query = urlencode({"expand": "1", "title": title, "body": body}, quote_via=quote) url = f"https://github.com/{repo}/compare/{refs}?{query}" if len(url) > MAX_PR_URL_LENGTH: @@ -390,9 +402,18 @@ def check_pushed(repo_dir: Path, remote: str, branch: str) -> None: def find_open_pr(repo: str, head: str) -> str | None: - """Return the URL of the open pull request from head, or None if gh cannot find one.""" - command = ["gh", "pr", "list", "--repo", repo, "--head", head, "--state", "open"] - command += ["--json", "url", "--jq", ".[0].url // empty"] + """Return the URL of the open pull request from head, or None if gh cannot find one. + + head is BRANCH, or OWNER:BRANCH for a branch in a fork of repo. + """ + owner, _, branch = head.rpartition(":") + command = ["gh", "pr", "list", "--repo", repo, "--head", branch, "--state", "open"] + if owner: + # gh filters by the branch name only. The jq filter also checks the owner of the branch. + login = f'select(.headRepositoryOwner.login | ascii_downcase == "{owner.lower()}")' + command += ["--json", "url,headRepositoryOwner", "--jq", f"[.[] | {login}][0].url // empty"] + else: + command += ["--json", "url", "--jq", ".[0].url // empty"] try: result = subprocess.run(command, capture_output=True, encoding="utf-8", timeout=60) except (OSError, subprocess.TimeoutExpired): @@ -404,17 +425,22 @@ def find_open_pr(repo: str, head: str) -> str | None: def pr_request(args: argparse.Namespace) -> Request: """Make the request that opens the GitHub form for a new pull request.""" repo_dir: Path = args.repo_dir - repo: str = args.repo or github_repo(git(repo_dir, "remote", "get-url", args.remote)) + remote_url = git(repo_dir, "remote", "get-url", args.remote) + repo: str = args.repo or github_repo(remote_url) if not REPO_RE.fullmatch(repo): raise HandoffError(f"{repo!r} is not a repository name of the form OWNER/NAME.") head: str = args.head or current_branch(repo_dir) check_pushed(repo_dir, args.remote, head) + # A pull request to another repository, for example from a fork to its upstream + # repository, names the branch as OWNER:BRANCH. + owner = fork_owner(remote_url, repo) + head_ref = head if owner is None else f"{owner}:{head}" try: body: str = args.body_file.read_text(encoding="utf-8") if args.body_file else args.body except OSError as exc: raise HandoffError(f"Cannot read {args.body_file}: {exc}") from exc - url = compare_url(repo, args.base, head, args.title, body) - return Request(f"Create pull request: {args.title}", url, PR_STEPS, pr=(repo, head)) + url = compare_url(repo, args.base, head_ref, args.title, body) + return Request(f"Create pull request: {args.title}", url, PR_STEPS, pr=(repo, head_ref)) class Dialog: diff --git a/skills/deploy-handoff/test_handoff.py b/skills/deploy-handoff/test_handoff.py index a18d8b7..645c83c 100644 --- a/skills/deploy-handoff/test_handoff.py +++ b/skills/deploy-handoff/test_handoff.py @@ -525,11 +525,58 @@ class CompareUrlTest(unittest.TestCase): query = parse_qs(parts.query, keep_blank_values=True, strict_parsing=True) self.assertEqual(query, {"expand": ["1"], "title": [title], "body": [body]}) + @given(REPO, st.none() | branches(), OWNER, branches()) + def test_keeps_the_owner_of_a_fork_branch( + self, repo: str, base: str | None, owner: str, branch: str + ) -> None: + # GitHub compares a branch of a fork as OWNER:BRANCH. + url = handoff.compare_url(repo, base, f"{owner}:{branch}", "t", "b") + head = urlsplit(url).path.removeprefix(f"/{repo}/compare/").split("...")[-1] + self.assertTrue(head.startswith(f"{owner}:")) + self.assertEqual(unquote(head), f"{owner}:{branch}") + def test_refuses_long_urls(self) -> None: with self.assertRaises(HandoffError): handoff.compare_url("o/r", None, "b", "t", "x" * handoff.MAX_PR_URL_LENGTH) +class ForkOwnerTest(unittest.TestCase): + @given(OWNER, NAME, st.sampled_from(REMOTE_FORMS), REPO) + def test_names_the_owner_only_for_another_repo( + self, owner: str, name: str, form: str, other: str + ) -> None: + remote = form.format(f"{owner}/{name}") + self.assertIsNone(handoff.fork_owner(remote, f"{owner}/{name}".swapcase())) + if other.lower() != f"{owner}/{name}".lower(): + self.assertEqual(handoff.fork_owner(remote, other), owner) + + def test_a_remote_that_is_not_on_github_has_no_owner(self) -> None: + self.assertIsNone(handoff.fork_owner("C:/repos/r.git", "o/r")) + + +class FindOpenPrTest(unittest.TestCase): + def find(self, head: str) -> tuple[str | None, list[str]]: + result = subprocess.CompletedProcess[str]([], 0, "https://github.com/up/r/pull/7\n", "") + with mock.patch("subprocess.run", return_value=result) as run: + url = handoff.find_open_pr("up/r", head) + command: list[str] = run.call_args.args[0] + return url, command + + def test_a_branch_of_the_same_repo(self) -> None: + url, command = self.find("main") + self.assertEqual(url, "https://github.com/up/r/pull/7") + self.assertEqual(command[command.index("--head") + 1], "main") + self.assertEqual(command[command.index("--jq") + 1], ".[0].url // empty") + + def test_a_branch_of_a_fork_checks_the_owner(self) -> None: + # gh filters by the branch name only. The jq filter checks the owner of the branch. + url, command = self.find("Me:fix/x") + self.assertEqual(url, "https://github.com/up/r/pull/7") + self.assertEqual(command[command.index("--head") + 1], "fix/x") + self.assertIn("headRepositoryOwner", command[command.index("--json") + 1]) + self.assertIn('== "me"', command[command.index("--jq") + 1]) + + class GitTest(unittest.TestCase): def setUp(self) -> None: folder = tempfile.TemporaryDirectory(ignore_cleanup_errors=True) @@ -686,6 +733,21 @@ class MainTest(unittest.TestCase): ) find_open_pr.assert_called_once_with("o/r", "main") + def test_pr_from_a_fork_names_the_owner_of_the_branch(self) -> None: + work = make_repo(self.root) + run_git("-C", str(work), "remote", "set-url", "origin", "https://github.com/me/r.git") + # check_pushed asks the remote. GitHub does not have this test repository. + self.patch("check_pushed", mock.MagicMock()) + find_open_pr = self.patch("find_open_pr", mock.MagicMock(return_value=None)) + code, result = self.run_main( + "pr", "--repo", "up/r", "--repo-dir", str(work), "--title", "Fix X", "--body", "Why" + ) + self.assertEqual((code, result["status"]), (0, "done")) + self.open_page.assert_called_once_with( + "https://github.com/up/r/compare/me:main?expand=1&title=Fix%20X&body=Why" + ) + find_open_pr.assert_called_once_with("up/r", "me:main") + def test_browser_that_runs(self) -> None: self.patch("debug_version", mock.MagicMock(return_value={"Browser": "Chrome/140.0"})) start = self.patch("start_browser", mock.MagicMock())