From 6972410b7f0980c931d89fb20ec0a5ea64fc1e6f Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Wed, 9 Sep 2026 20:11:25 +0000 Subject: [PATCH 1/2] Add shared publish.py script (Python rewrite of publish.sh) Replace the Bash publish.sh with a Python implementation that preserves the same subcommand interface (preflight, push, check-existing, create-pr, create-mr, save-metadata) and exit code contract (0/1/3/4/5). The rewrite gains native JSON handling via json.dumps, eliminating the manual json_escape function, NUL-delimited sorting, and temp-file pipelines that accumulated complexity in the Bash version. - Add _shared/scripts/publish.py with argparse CLI, subprocess.run for git/gh/glab operations, and structured JSON output - Add _shared/scripts/test_publish.py with 56 unittest tests covering argument parsing, JSON encoding edge cases (newlines, quotes, backslashes, control chars, leading-zero strings, Unicode), exit code contracts, preflight output structure, and error handling paths - Update all 6 consuming skill files to use python3 "$PUBLISH_SCRIPT" - Update AGENTS.md to reference publish.py in shared scripts section - PATCH-bump bugfix 0.8.0->0.8.1, design 0.9.1->0.9.2, docs-writer 0.3.1->0.3.2, e2e 0.7.0->0.7.1, implement 0.9.0->0.9.1, prd 0.9.1->0.9.2 Assisted-by: Claude --- AGENTS.md | 4 +- _shared/scripts/publish.py | 621 +++++++++++++++++++++++++ _shared/scripts/test_publish.py | 771 ++++++++++++++++++++++++++++++++ bugfix/SKILL.md | 2 +- bugfix/skills/pr.md | 151 +++---- design/SKILL.md | 2 +- design/skills/publish.md | 95 ++-- docs-writer/SKILL.md | 2 +- docs-writer/skills/create-mr.md | 127 ++++-- e2e/SKILL.md | 2 +- e2e/skills/publish.md | 130 ++++-- implement/SKILL.md | 2 +- implement/skills/publish.md | 119 +++-- prd/SKILL.md | 2 +- prd/skills/publish.md | 112 ++++- 15 files changed, 1895 insertions(+), 247 deletions(-) create mode 100644 _shared/scripts/publish.py create mode 100644 _shared/scripts/test_publish.py diff --git a/AGENTS.md b/AGENTS.md index 2a035646..ce5f3659 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,6 +83,7 @@ _shared/ sizing-rubric.md # Shared sizing definitions (T-shirt sizes, heuristics, team effort guidance) scripts/ provenance.py # Capture/render CLI (used by prd and design provenance recipes) + publish.py # Deterministic publish operations (push, PR/MR, metadata) recipes/ capture-provenance-event.md # Append session-local provenance on doc-mutating phases phase-override-resolution.md # Project-level phase override lookup and activation @@ -196,7 +197,8 @@ ai-workflows/ │ ├── review-protocol.md # Shared code review criteria and finding format │ ├── sizing-rubric.md # Shared sizing definitions and heuristics │ ├── scripts/ -│ │ └── provenance.py # Capture/render CLI for prd/design provenance +│ │ ├── provenance.py # Capture/render CLI for prd/design provenance +│ │ └── publish.py # Deterministic publish operations (push, PR/MR, metadata) │ └── recipes/ │ ├── capture-provenance-event.md │ ├── phase-override-resolution.md # Project-level phase override lookup diff --git a/_shared/scripts/publish.py b/_shared/scripts/publish.py new file mode 100644 index 00000000..75dec1ae --- /dev/null +++ b/_shared/scripts/publish.py @@ -0,0 +1,621 @@ +#!/usr/bin/env python3 +"""Deterministic publish operations for ai-workflows. + +Provides reusable subcommands for the publish/PR/MR phase of multiple +workflows (bugfix, implement, e2e, prd, design, docs-writer). Each +subcommand handles one discrete, deterministic operation -- the calling +skill file retains ownership of AI-dependent work (PR body generation, +cross-cutting review, user confirmation prompts). + +Subcommands: + preflight Pre-flight checks (auth, branch, uncommitted changes) + push Push a branch to a remote + check-existing Check whether a PR/MR already exists for a branch + create-pr Create a GitHub pull request via gh CLI + create-mr Create a GitLab merge request via glab CLI + save-metadata Write publish-metadata.json + +Usage: + publish.py preflight [--platform github|gitlab] + publish.py push --remote --branch + publish.py check-existing --repo --head + [--platform github|gitlab] + publish.py create-pr --repo --base --head + --title [--body-file <path>] [--body <text>] + [--draft] [--labels <csv>] + publish.py create-mr --project <path> --source <branch> + --target <branch> --title <title> [--description <text>] + [--desc-file <path>] [--draft] [--head <project>] + publish.py save-metadata --file <path> [--pair key=value ...] + +Exit codes: + 0 -- success + 1 -- missing argument or configuration error + 3 -- push failed + 4 -- PR/MR creation failed + 5 -- existing PR/MR found (check-existing only; prints JSON on stdout) +""" + +from __future__ import annotations + +import argparse +import json +import os +import subprocess +import sys +from pathlib import Path +from typing import Any, NoReturn + + +# --------------------------------------------------------------------------- +# Exit codes +# --------------------------------------------------------------------------- + +EXIT_SUCCESS = 0 +EXIT_ARG_ERROR = 1 +EXIT_PUSH_FAIL = 3 +EXIT_CREATE_FAIL = 4 +EXIT_EXISTING_FOUND = 5 + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def info(msg: str) -> None: + """Print an informational message to stderr.""" + print(f"INFO: {msg}", file=sys.stderr) + + +def fail(msg: str, code: int = EXIT_ARG_ERROR) -> NoReturn: + """Print an error message to stderr and exit.""" + print(f"ERROR: {msg}", file=sys.stderr) + sys.exit(code) + + +def run( + cmd: list[str], + *, + capture: bool = True, + check: bool = False, +) -> subprocess.CompletedProcess[str]: + """Run a subprocess with text output.""" + return subprocess.run( + cmd, + capture_output=capture, + text=True, + check=check, + ) + + +# --------------------------------------------------------------------------- +# Subcommand: preflight +# --------------------------------------------------------------------------- + +def cmd_preflight(args: argparse.Namespace) -> int: + """Run pre-flight checks: auth, branch, and working-tree cleanliness. + + Prints a JSON object on stdout with auth_ok, auth_user, branch, + has_uncommitted, has_staged, has_untracked, and platform fields. + """ + platform = args.platform + + if platform not in ("github", "gitlab"): + fail(f"preflight: invalid platform: {platform} " + f"(expected github or gitlab)") + + auth_ok = False + auth_user = "" + + # -- Auth check -- + if platform == "github": + result = run(["gh", "auth", "status"]) + if result.returncode == 0: + auth_ok = True + r = run(["gh", "api", "user", "--jq", ".login"]) + if r.returncode == 0 and r.stdout.strip(): + auth_user = r.stdout.strip() + else: + # GitHub App / bot -- try installation endpoint + r = run([ + "gh", "api", "/installation/repositories", + "--jq", ".repositories[0].owner.login", + ]) + if r.returncode == 0 and r.stdout.strip(): + auth_user = r.stdout.strip() + else: # gitlab + result = run(["glab", "auth", "status"]) + if result.returncode == 0: + auth_ok = True + r = run(["glab", "api", "user", "--jq", ".username"]) + if r.returncode == 0 and r.stdout.strip(): + auth_user = r.stdout.strip() + + # -- Branch -- + r = run(["git", "branch", "--show-current"]) + branch = r.stdout.strip() if r.returncode == 0 else "" + + # -- Remote -- + remote = "" + r = run(["git", "remote"]) + if r.returncode == 0: + remotes = r.stdout.strip().splitlines() + if remotes: + # Prefer "fork" if it exists, else first remote + remote = "fork" if "fork" in remotes else remotes[0] + + # -- Uncommitted changes -- + has_uncommitted = run(["git", "diff", "--quiet"]).returncode != 0 + has_staged = run(["git", "diff", "--cached", "--quiet"]).returncode != 0 + + r = run(["git", "ls-files", "--others", "--exclude-standard"]) + has_untracked = bool(r.returncode == 0 and r.stdout.strip()) + + output: dict[str, Any] = { + "auth_ok": auth_ok, + "auth_user": auth_user, + "branch": branch, + "remote": remote, + "has_uncommitted": has_uncommitted, + "has_staged": has_staged, + "has_untracked": has_untracked, + "platform": platform, + } + + print(json.dumps(output, indent=2)) + return EXIT_SUCCESS + + +# --------------------------------------------------------------------------- +# Subcommand: push +# --------------------------------------------------------------------------- + +def cmd_push(args: argparse.Namespace) -> int: + """Push a branch to the specified remote with upstream tracking (-u).""" + remote = args.remote + branch = args.branch + + if not remote: + fail("Missing required argument: --remote") + if not branch: + fail("Missing required argument: --branch") + + # Verify the remote exists + r = run(["git", "remote", "get-url", remote]) + if r.returncode != 0: + remotes_r = run(["git", "remote"]) + available = remotes_r.stdout.strip().replace("\n", " ") if remotes_r.returncode == 0 else "(none)" + fail(f"push: remote '{remote}' does not exist. " + f"Available remotes: {available}", EXIT_PUSH_FAIL) + + info(f"Pushing {branch} to {remote}...") + result = run(["git", "push", "-u", remote, branch], capture=False) + if result.returncode != 0: + fail(f"push: git push failed (remote={remote}, branch={branch})", + EXIT_PUSH_FAIL) + + info(f"Push successful: {remote}/{branch}") + return EXIT_SUCCESS + + +# --------------------------------------------------------------------------- +# Subcommand: check-existing +# --------------------------------------------------------------------------- + +def cmd_check_existing(args: argparse.Namespace) -> int: + """Check whether an open PR (GitHub) or MR (GitLab) already exists. + + Exit code 0 if NO existing PR/MR found (safe to create one). + Exit code 5 if an existing PR/MR IS found (details as JSON on stdout). + """ + repo = args.repo + head = args.head + platform = args.platform + + if not repo: + fail("Missing required argument: --repo") + if not head: + fail("Missing required argument: --head") + + if platform == "github": + return _check_existing_github(repo, head) + elif platform == "gitlab": + return _check_existing_gitlab(repo, head) + else: + fail(f"check-existing: invalid platform: {platform}") + return EXIT_ARG_ERROR # unreachable; satisfies type checker + + +def _check_existing_github(repo: str, head: str) -> int: + """GitHub check-existing: use gh pr list with headRepositoryOwner filtering.""" + if ":" in head: + # owner:branch format -- gh pr list --head does not support this + # syntax. Search by branch name and filter by head repo owner. + head_owner, head_branch = head.split(":", 1) + r = run([ + "gh", "pr", "list", + "--repo", repo, + "--head", head_branch, + "--json", "number,url,headRepositoryOwner", + ]) + if r.returncode != 0: + fail("check-existing: GitHub API query failed. " + "Check gh auth status.") + + try: + prs = json.loads(r.stdout) if r.stdout.strip() else [] + except json.JSONDecodeError: + fail("check-existing: failed to parse GitHub API response") + return EXIT_ARG_ERROR # unreachable + + # Filter by headRepositoryOwner + matching = [ + pr for pr in prs + if isinstance(pr.get("headRepositoryOwner"), dict) + and pr["headRepositoryOwner"].get("login") == head_owner + ] + if matching: + result = {"number": matching[0]["number"], "url": matching[0]["url"]} + print(json.dumps(result, indent=2)) + return EXIT_EXISTING_FOUND + else: + r = run([ + "gh", "pr", "list", + "--repo", repo, + "--head", head, + "--json", "number,url", + ]) + if r.returncode != 0: + fail("check-existing: GitHub API query failed. " + "Check gh auth status.") + + try: + prs = json.loads(r.stdout) if r.stdout.strip() else [] + except json.JSONDecodeError: + fail("check-existing: failed to parse GitHub API response") + return EXIT_ARG_ERROR # unreachable + + if prs: + result = {"number": prs[0]["number"], "url": prs[0]["url"]} + print(json.dumps(result, indent=2)) + return EXIT_EXISTING_FOUND + + # No existing PR found + info(f"No existing PR/MR found for head={head} on {repo}") + return EXIT_SUCCESS + + +def _check_existing_gitlab(repo: str, head: str) -> int: + """GitLab check-existing: use glab mr list with source_project_id filtering.""" + source_branch = head + source_project = "" + + if ":" in head: + # project:branch format -- extract source project for cross-fork filtering + source_project, source_branch = head.split(":", 1) + + r = run([ + "glab", "mr", "list", + "--repo", repo, + "--source-branch", source_branch, + "--output", "json", + ]) + if r.returncode != 0: + fail("check-existing: GitLab API query failed. " + "Check glab auth status.") + + try: + mrs = json.loads(r.stdout) if r.stdout.strip() else [] + except json.JSONDecodeError: + fail("check-existing: failed to parse GitLab API response") + return EXIT_ARG_ERROR # unreachable + + if source_project: + # Resolve the fork's numeric project ID to filter by source_project_id + encoded = source_project.replace("/", "%2F") + r = run(["glab", "api", f"projects/{encoded}", "--jq", ".id"]) + if r.returncode != 0 or not r.stdout.strip(): + fail(f"check-existing: could not resolve project ID " + f"for '{source_project}'") + + try: + project_id = int(r.stdout.strip()) + except ValueError: + fail(f"check-existing: invalid project ID from API: " + f"{r.stdout.strip()!r}") + return EXIT_ARG_ERROR # unreachable + + matching = [ + mr for mr in mrs + if mr.get("source_project_id") == project_id + ] + if matching: + print(json.dumps(matching[0], indent=2)) + return EXIT_EXISTING_FOUND + else: + if mrs: + print(json.dumps(mrs[0], indent=2)) + return EXIT_EXISTING_FOUND + + info(f"No existing PR/MR found for head={head} on {repo}") + return EXIT_SUCCESS + + +# --------------------------------------------------------------------------- +# Subcommand: create-pr +# --------------------------------------------------------------------------- + +def cmd_create_pr(args: argparse.Namespace) -> int: + """Create a GitHub pull request via the gh CLI.""" + if not args.base: + fail("Missing required argument: --base") + if not args.head: + fail("Missing required argument: --head") + if not args.title: + fail("Missing required argument: --title") + + cmd: list[str] = ["gh", "pr", "create"] + + if args.draft: + cmd.append("--draft") + + if args.repo: + cmd.extend(["--repo", args.repo]) + + cmd.extend(["--base", args.base, "--head", args.head, "--title", args.title]) + + if args.body_file: + if not Path(args.body_file).is_file(): + fail(f"create-pr: body file not found: {args.body_file}") + cmd.extend(["--body-file", args.body_file]) + elif args.body is not None: + cmd.extend(["--body", args.body]) + else: + cmd.extend(["--body", ""]) + + if args.labels: + cmd.extend(["--label", args.labels]) + + info(f"Creating PR: {args.title}") + result = run(cmd, capture=True) + if result.returncode != 0: + if result.stderr: + print(result.stderr, file=sys.stderr, end="") + sys.exit(EXIT_CREATE_FAIL) + + pr_url = result.stdout.strip() + if pr_url: + print(pr_url) + info(f"PR created: {pr_url}") + return EXIT_SUCCESS + + +# --------------------------------------------------------------------------- +# Subcommand: create-mr +# --------------------------------------------------------------------------- + +def cmd_create_mr(args: argparse.Namespace) -> int: + """Create a GitLab merge request via the glab CLI.""" + if not args.source: + fail("Missing required argument: --source") + if not args.target: + fail("Missing required argument: --target") + if not args.title: + fail("Missing required argument: --title") + + cmd: list[str] = ["glab", "mr", "create", "--yes"] + + if args.draft: + cmd.append("--draft") + + if args.project: + cmd.extend(["--repo", args.project]) + + if args.head: + cmd.extend(["--head", args.head]) + + cmd.extend([ + "--source-branch", args.source, + "--target-branch", args.target, + "--title", args.title, + ]) + + description = args.description or "" + if args.desc_file: + desc_path = Path(args.desc_file) + if not desc_path.is_file(): + fail(f"create-mr: description file not found: {args.desc_file}") + description = desc_path.read_text(encoding="utf-8") + + if description: + cmd.extend(["--description", description]) + + info(f"Creating MR: {args.title}") + result = run(cmd, capture=True) + if result.returncode != 0: + if result.stderr: + print(result.stderr, file=sys.stderr, end="") + sys.exit(EXIT_CREATE_FAIL) + + mr_url = result.stdout.strip() + if mr_url: + print(mr_url) + info(f"MR created: {mr_url}") + return EXIT_SUCCESS + + +# --------------------------------------------------------------------------- +# Subcommand: save-metadata +# --------------------------------------------------------------------------- + +def cmd_save_metadata(args: argparse.Namespace) -> int: + """Write a JSON metadata file from key=value pairs.""" + file_path = args.file + pairs: list[str] = args.pair or [] + + if not file_path: + fail("Missing required argument: --file") + if not pairs: + fail("save-metadata: no key=value pairs provided") + + data: dict[str, str] = {} + for pair in pairs: + if "=" not in pair: + fail(f"save-metadata: unexpected argument: {pair} " + f"(expected key=value)") + key, _, value = pair.partition("=") + # All values stored as JSON strings to avoid leading-zero truncation + # (e.g., "007" -> 7) and to keep the output type-stable. + data[key] = value + + # Sort keys for stable output (Python dicts preserve insertion order) + sorted_data = dict(sorted(data.items())) + + # Create parent directory if needed + out_path = Path(file_path) + out_path.parent.mkdir(parents=True, exist_ok=True) + + # Write with json.dumps -- proper encoding of all special characters + out_path.write_text( + json.dumps(sorted_data, indent=2, ensure_ascii=False) + "\n", + encoding="utf-8", + ) + + info(f"Metadata saved to {file_path}") + return EXIT_SUCCESS + + +# --------------------------------------------------------------------------- +# Argument parsing +# --------------------------------------------------------------------------- + +def build_parser() -> argparse.ArgumentParser: + """Build the top-level argument parser with subcommand parsers.""" + parser = argparse.ArgumentParser( + description="Deterministic publish operations for ai-workflows.", + ) + subparsers = parser.add_subparsers(dest="subcommand") + + # -- preflight -- + p_preflight = subparsers.add_parser( + "preflight", + help="Pre-flight checks (auth, branch, uncommitted changes)", + ) + p_preflight.add_argument( + "--platform", + choices=["github", "gitlab"], + default="github", + help="Which CLI to check (default: github)", + ) + + # -- push -- + p_push = subparsers.add_parser( + "push", + help="Push a branch to a remote", + ) + p_push.add_argument("--remote", required=True, help="Git remote name") + p_push.add_argument("--branch", required=True, help="Branch name to push") + + # -- check-existing -- + p_check = subparsers.add_parser( + "check-existing", + help="Check whether a PR/MR already exists for a branch", + ) + p_check.add_argument("--repo", required=True, help="Target repository") + p_check.add_argument("--head", required=True, help="Branch or owner:branch") + p_check.add_argument( + "--platform", + choices=["github", "gitlab"], + default="github", + help="Which platform (default: github)", + ) + + # -- create-pr -- + p_pr = subparsers.add_parser( + "create-pr", + help="Create a GitHub pull request via gh CLI", + ) + p_pr.add_argument("--repo", default="", help="Target repository") + p_pr.add_argument("--base", required=True, help="Base branch") + p_pr.add_argument("--head", required=True, help="Head ref") + p_pr.add_argument("--title", required=True, help="PR title") + p_pr.add_argument("--body-file", default="", help="Path to body file") + p_pr.add_argument("--body", default=None, help="Inline PR body text") + p_pr.add_argument( + "--draft", action="store_true", default=True, + help="Create as draft PR (default: true)", + ) + p_pr.add_argument( + "--no-draft", dest="draft", action="store_false", + help="Create as non-draft PR", + ) + p_pr.add_argument("--labels", default="", help="Comma-separated labels") + + # -- create-mr -- + p_mr = subparsers.add_parser( + "create-mr", + help="Create a GitLab merge request via glab CLI", + ) + p_mr.add_argument("--project", default="", help="Upstream project path") + p_mr.add_argument("--source", required=True, help="Source branch") + p_mr.add_argument("--target", required=True, help="Target branch") + p_mr.add_argument("--title", required=True, help="MR title") + p_mr.add_argument("--description", default="", help="MR description text") + p_mr.add_argument("--desc-file", default="", help="Path to description file") + p_mr.add_argument( + "--draft", action="store_true", default=True, + help="Create as draft MR (default: true)", + ) + p_mr.add_argument( + "--no-draft", dest="draft", action="store_false", + help="Create as non-draft MR", + ) + p_mr.add_argument("--head", default="", help="Fork project path") + + # -- save-metadata -- + p_meta = subparsers.add_parser( + "save-metadata", + help="Write publish-metadata.json", + ) + p_meta.add_argument("--file", required=True, help="Output file path") + p_meta.add_argument( + "pair", nargs="*", metavar="key=value", + help="Key=value pairs (positional, repeatable)", + ) + + return parser + + +# --------------------------------------------------------------------------- +# Main +# --------------------------------------------------------------------------- + +SUBCOMMAND_MAP = { + "preflight": cmd_preflight, + "push": cmd_push, + "check-existing": cmd_check_existing, + "create-pr": cmd_create_pr, + "create-mr": cmd_create_mr, + "save-metadata": cmd_save_metadata, +} + + +def main(argv: list[str] | None = None) -> int: + """Entry point.""" + parser = build_parser() + args = parser.parse_args(argv) + + if not args.subcommand: + parser.print_help(sys.stderr) + return EXIT_ARG_ERROR + + handler = SUBCOMMAND_MAP.get(args.subcommand) + if handler is None: + fail(f"Unknown subcommand: {args.subcommand}. " + f"Run with --help for usage.") + + return handler(args) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/_shared/scripts/test_publish.py b/_shared/scripts/test_publish.py new file mode 100644 index 00000000..7aaf0719 --- /dev/null +++ b/_shared/scripts/test_publish.py @@ -0,0 +1,771 @@ +#!/usr/bin/env python3 +"""Tests for _shared/scripts/publish.py.""" + +from __future__ import annotations + +import importlib.util +import json +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +_SCRIPT = Path(__file__).resolve().parent / "publish.py" +_spec = importlib.util.spec_from_file_location("publish", _SCRIPT) +assert _spec and _spec.loader +publish = importlib.util.module_from_spec(_spec) +sys.modules["publish"] = publish +_spec.loader.exec_module(publish) + + +# --------------------------------------------------------------------------- +# Argument parsing tests +# --------------------------------------------------------------------------- + + +class TestParseArgs(unittest.TestCase): + """Verify argparse configuration for each subcommand.""" + + def test_preflight_defaults(self) -> None: + parser = publish.build_parser() + args = parser.parse_args(["preflight"]) + self.assertEqual(args.subcommand, "preflight") + self.assertEqual(args.platform, "github") + + def test_preflight_gitlab(self) -> None: + parser = publish.build_parser() + args = parser.parse_args(["preflight", "--platform", "gitlab"]) + self.assertEqual(args.platform, "gitlab") + + def test_push_args(self) -> None: + parser = publish.build_parser() + args = parser.parse_args(["push", "--remote", "fork", "--branch", "feat/x"]) + self.assertEqual(args.subcommand, "push") + self.assertEqual(args.remote, "fork") + self.assertEqual(args.branch, "feat/x") + + def test_push_missing_remote(self) -> None: + parser = publish.build_parser() + with self.assertRaises(SystemExit): + parser.parse_args(["push", "--branch", "feat/x"]) + + def test_check_existing_args(self) -> None: + parser = publish.build_parser() + args = parser.parse_args([ + "check-existing", "--repo", "acme/proj", "--head", "feat/x", + ]) + self.assertEqual(args.subcommand, "check-existing") + self.assertEqual(args.repo, "acme/proj") + self.assertEqual(args.head, "feat/x") + self.assertEqual(args.platform, "github") + + def test_check_existing_gitlab(self) -> None: + parser = publish.build_parser() + args = parser.parse_args([ + "check-existing", "--repo", "group/proj", + "--head", "feat/x", "--platform", "gitlab", + ]) + self.assertEqual(args.platform, "gitlab") + + def test_create_pr_args(self) -> None: + parser = publish.build_parser() + args = parser.parse_args([ + "create-pr", "--base", "main", "--head", "user:feat/x", + "--title", "Fix bug", "--repo", "acme/proj", + ]) + self.assertEqual(args.subcommand, "create-pr") + self.assertEqual(args.base, "main") + self.assertEqual(args.head, "user:feat/x") + self.assertEqual(args.title, "Fix bug") + self.assertTrue(args.draft) + + def test_create_pr_no_draft(self) -> None: + parser = publish.build_parser() + args = parser.parse_args([ + "create-pr", "--base", "main", "--head", "feat/x", + "--title", "Fix", "--no-draft", + ]) + self.assertFalse(args.draft) + + def test_create_mr_args(self) -> None: + parser = publish.build_parser() + args = parser.parse_args([ + "create-mr", "--source", "docs/fix", "--target", "main", + "--title", "Update docs", + ]) + self.assertEqual(args.subcommand, "create-mr") + self.assertEqual(args.source, "docs/fix") + self.assertEqual(args.target, "main") + self.assertEqual(args.title, "Update docs") + self.assertTrue(args.draft) + + def test_create_mr_no_draft(self) -> None: + parser = publish.build_parser() + args = parser.parse_args([ + "create-mr", "--source", "x", "--target", "main", + "--title", "T", "--no-draft", + ]) + self.assertFalse(args.draft) + + def test_save_metadata_args(self) -> None: + parser = publish.build_parser() + args = parser.parse_args([ + "save-metadata", "--file", "out.json", + "key1=val1", "key2=val2", + ]) + self.assertEqual(args.subcommand, "save-metadata") + self.assertEqual(args.file, "out.json") + self.assertEqual(args.pair, ["key1=val1", "key2=val2"]) + + def test_no_subcommand(self) -> None: + result = publish.main([]) + self.assertEqual(result, publish.EXIT_ARG_ERROR) + + +# --------------------------------------------------------------------------- +# save-metadata tests +# --------------------------------------------------------------------------- + + +class TestSaveMetadata(unittest.TestCase): + """Verify save-metadata JSON output and edge cases.""" + + def test_basic_metadata(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "meta.json") + code = publish.main([ + "save-metadata", "--file", out, + "repo=acme/proj", + "branch=feat/x", + ]) + self.assertEqual(code, 0) + data = json.loads(Path(out).read_text(encoding="utf-8")) + self.assertEqual(data["repo"], "acme/proj") + self.assertEqual(data["branch"], "feat/x") + + def test_keys_sorted(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "meta.json") + publish.main([ + "save-metadata", "--file", out, + "z_key=last", + "a_key=first", + "m_key=middle", + ]) + data = json.loads(Path(out).read_text(encoding="utf-8")) + keys = list(data.keys()) + self.assertEqual(keys, ["a_key", "m_key", "z_key"]) + + def test_newlines_in_values(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "meta.json") + publish.main([ + "save-metadata", "--file", out, + "body=line1\nline2\nline3", + ]) + data = json.loads(Path(out).read_text(encoding="utf-8")) + self.assertEqual(data["body"], "line1\nline2\nline3") + + def test_quotes_and_backslashes(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "meta.json") + publish.main([ + "save-metadata", "--file", out, + 'msg=He said "hello\\world"', + ]) + data = json.loads(Path(out).read_text(encoding="utf-8")) + self.assertEqual(data["msg"], 'He said "hello\\world"') + + def test_control_characters(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "meta.json") + publish.main([ + "save-metadata", "--file", out, + "tab=a\tb", + "cr=a\rb", + ]) + raw = Path(out).read_text(encoding="utf-8") + data = json.loads(raw) + self.assertEqual(data["tab"], "a\tb") + self.assertEqual(data["cr"], "a\rb") + + def test_leading_zero_strings(self) -> None: + """Values with leading zeros must remain strings, not become integers.""" + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "meta.json") + publish.main([ + "save-metadata", "--file", out, + "pr_number=007", + ]) + data = json.loads(Path(out).read_text(encoding="utf-8")) + self.assertIsInstance(data["pr_number"], str) + self.assertEqual(data["pr_number"], "007") + + def test_unicode_values(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "meta.json") + publish.main([ + "save-metadata", "--file", out, + "name=Renée", + ]) + data = json.loads(Path(out).read_text(encoding="utf-8")) + self.assertEqual(data["name"], "Renée") + + def test_creates_parent_directory(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "nested", "dir", "meta.json") + code = publish.main([ + "save-metadata", "--file", out, + "key=val", + ]) + self.assertEqual(code, 0) + self.assertTrue(Path(out).is_file()) + + def test_no_pairs_fails(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "meta.json") + with self.assertRaises(SystemExit) as ctx: + publish.main(["save-metadata", "--file", out]) + self.assertEqual(ctx.exception.code, publish.EXIT_ARG_ERROR) + + def test_invalid_pair_format_fails(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "meta.json") + with self.assertRaises(SystemExit) as ctx: + publish.main([ + "save-metadata", "--file", out, + "noequalssign", + ]) + self.assertEqual(ctx.exception.code, publish.EXIT_ARG_ERROR) + + def test_empty_value_is_valid(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "meta.json") + publish.main([ + "save-metadata", "--file", out, + "empty=", + ]) + data = json.loads(Path(out).read_text(encoding="utf-8")) + self.assertEqual(data["empty"], "") + + def test_value_with_equals(self) -> None: + """Values containing '=' should work (only first '=' is the delimiter).""" + with tempfile.TemporaryDirectory() as tmp: + out = os.path.join(tmp, "meta.json") + publish.main([ + "save-metadata", "--file", out, + "url=https://example.com?a=1&b=2", + ]) + data = json.loads(Path(out).read_text(encoding="utf-8")) + self.assertEqual(data["url"], "https://example.com?a=1&b=2") + + +# --------------------------------------------------------------------------- +# Exit code contract tests +# --------------------------------------------------------------------------- + + +class TestExitCodes(unittest.TestCase): + """Verify the documented exit code contract.""" + + def test_exit_code_constants(self) -> None: + self.assertEqual(publish.EXIT_SUCCESS, 0) + self.assertEqual(publish.EXIT_ARG_ERROR, 1) + self.assertEqual(publish.EXIT_PUSH_FAIL, 3) + self.assertEqual(publish.EXIT_CREATE_FAIL, 4) + self.assertEqual(publish.EXIT_EXISTING_FOUND, 5) + + def test_push_missing_remote_exits_1(self) -> None: + with self.assertRaises(SystemExit) as ctx: + publish.main(["push", "--remote", "", "--branch", "x"]) + self.assertEqual(ctx.exception.code, publish.EXIT_ARG_ERROR) + + def test_push_missing_branch_exits_1(self) -> None: + with self.assertRaises(SystemExit) as ctx: + publish.main(["push", "--remote", "origin", "--branch", ""]) + self.assertEqual(ctx.exception.code, publish.EXIT_ARG_ERROR) + + @mock.patch.object(publish, "run") + def test_push_nonexistent_remote_exits_3(self, mock_run: mock.Mock) -> None: + # git remote get-url fails + mock_run.side_effect = [ + subprocess.CompletedProcess([], 1, "", ""), # git remote get-url + subprocess.CompletedProcess([], 0, "", ""), # git remote (listing) + ] + with self.assertRaises(SystemExit) as ctx: + publish.main(["push", "--remote", "nope", "--branch", "x"]) + self.assertEqual(ctx.exception.code, publish.EXIT_PUSH_FAIL) + + def test_create_pr_missing_base_exits_2(self) -> None: + """argparse exits with code 2 when required args are missing.""" + with self.assertRaises(SystemExit) as ctx: + publish.main(["create-pr", "--head", "x", "--title", "T"]) + self.assertEqual(ctx.exception.code, 2) + + @mock.patch.object(publish, "run") + def test_create_pr_failure_exits_4(self, mock_run: mock.Mock) -> None: + mock_run.return_value = subprocess.CompletedProcess( + [], 1, "", "Resource not accessible", + ) + with self.assertRaises(SystemExit) as ctx: + publish.main([ + "create-pr", "--base", "main", "--head", "feat/x", + "--title", "Fix bug", + ]) + self.assertEqual(ctx.exception.code, publish.EXIT_CREATE_FAIL) + + +# --------------------------------------------------------------------------- +# Preflight tests (with subprocess mocking) +# --------------------------------------------------------------------------- + + +class TestPreflight(unittest.TestCase): + """Verify preflight JSON output structure.""" + + @mock.patch.object(publish, "run") + def test_preflight_github_authenticated(self, mock_run: mock.Mock) -> None: + mock_run.side_effect = [ + # gh auth status + subprocess.CompletedProcess([], 0, "", ""), + # gh api user --jq .login + subprocess.CompletedProcess([], 0, "jsmith\n", ""), + # git branch --show-current + subprocess.CompletedProcess([], 0, "feat/x\n", ""), + # git remote + subprocess.CompletedProcess([], 0, "origin\nfork\n", ""), + # git diff --quiet + subprocess.CompletedProcess([], 1, "", ""), + # git diff --cached --quiet + subprocess.CompletedProcess([], 0, "", ""), + # git ls-files --others + subprocess.CompletedProcess([], 0, "", ""), + ] + + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + code = publish.main(["preflight"]) + + self.assertEqual(code, 0) + output = json.loads(buf.getvalue()) + self.assertTrue(output["auth_ok"]) + self.assertEqual(output["auth_user"], "jsmith") + self.assertEqual(output["branch"], "feat/x") + self.assertEqual(output["remote"], "fork") + self.assertTrue(output["has_uncommitted"]) + self.assertFalse(output["has_staged"]) + self.assertFalse(output["has_untracked"]) + self.assertEqual(output["platform"], "github") + + @mock.patch.object(publish, "run") + def test_preflight_not_authenticated(self, mock_run: mock.Mock) -> None: + mock_run.side_effect = [ + # gh auth status -> fail + subprocess.CompletedProcess([], 1, "", ""), + # git branch --show-current + subprocess.CompletedProcess([], 0, "main\n", ""), + # git remote + subprocess.CompletedProcess([], 0, "origin\n", ""), + # git diff --quiet + subprocess.CompletedProcess([], 0, "", ""), + # git diff --cached --quiet + subprocess.CompletedProcess([], 0, "", ""), + # git ls-files --others + subprocess.CompletedProcess([], 0, "newfile.txt\n", ""), + ] + + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + code = publish.main(["preflight"]) + + self.assertEqual(code, 0) + output = json.loads(buf.getvalue()) + self.assertFalse(output["auth_ok"]) + self.assertEqual(output["auth_user"], "") + self.assertTrue(output["has_untracked"]) + self.assertEqual(output["remote"], "origin") + + @mock.patch.object(publish, "run") + def test_preflight_gitlab(self, mock_run: mock.Mock) -> None: + mock_run.side_effect = [ + # glab auth status + subprocess.CompletedProcess([], 0, "", ""), + # glab api user --jq .username + subprocess.CompletedProcess([], 0, "gluser\n", ""), + # git branch --show-current + subprocess.CompletedProcess([], 0, "docs/fix\n", ""), + # git remote + subprocess.CompletedProcess([], 0, "origin\n", ""), + # git diff --quiet + subprocess.CompletedProcess([], 0, "", ""), + # git diff --cached --quiet + subprocess.CompletedProcess([], 0, "", ""), + # git ls-files --others + subprocess.CompletedProcess([], 0, "", ""), + ] + + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + code = publish.main(["preflight", "--platform", "gitlab"]) + + self.assertEqual(code, 0) + output = json.loads(buf.getvalue()) + self.assertTrue(output["auth_ok"]) + self.assertEqual(output["auth_user"], "gluser") + self.assertEqual(output["platform"], "gitlab") + + +# --------------------------------------------------------------------------- +# check-existing tests (with subprocess mocking) +# --------------------------------------------------------------------------- + + +class TestCheckExisting(unittest.TestCase): + """Verify check-existing behavior for both platforms.""" + + @mock.patch.object(publish, "run") + def test_github_no_existing_pr(self, mock_run: mock.Mock) -> None: + mock_run.return_value = subprocess.CompletedProcess( + [], 0, "[]", "", + ) + code = publish.main([ + "check-existing", "--repo", "acme/proj", "--head", "feat/x", + ]) + self.assertEqual(code, publish.EXIT_SUCCESS) + + @mock.patch.object(publish, "run") + def test_github_existing_pr_exits_5(self, mock_run: mock.Mock) -> None: + pr_data = [{"number": 42, "url": "https://github.com/acme/proj/pull/42"}] + mock_run.return_value = subprocess.CompletedProcess( + [], 0, json.dumps(pr_data), "", + ) + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + code = publish.main([ + "check-existing", "--repo", "acme/proj", "--head", "feat/x", + ]) + self.assertEqual(code, publish.EXIT_EXISTING_FOUND) + result = json.loads(buf.getvalue()) + self.assertEqual(result["number"], 42) + + @mock.patch.object(publish, "run") + def test_github_fork_owner_filter(self, mock_run: mock.Mock) -> None: + """Fork-aware check: filters by headRepositoryOwner.""" + pr_data = [ + { + "number": 10, + "url": "https://github.com/acme/proj/pull/10", + "headRepositoryOwner": {"login": "other-user"}, + }, + { + "number": 20, + "url": "https://github.com/acme/proj/pull/20", + "headRepositoryOwner": {"login": "jsmith"}, + }, + ] + mock_run.return_value = subprocess.CompletedProcess( + [], 0, json.dumps(pr_data), "", + ) + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + code = publish.main([ + "check-existing", "--repo", "acme/proj", + "--head", "jsmith:feat/x", + ]) + self.assertEqual(code, publish.EXIT_EXISTING_FOUND) + result = json.loads(buf.getvalue()) + self.assertEqual(result["number"], 20) + + @mock.patch.object(publish, "run") + def test_github_fork_no_match(self, mock_run: mock.Mock) -> None: + """Fork-aware check: no matching owner exits 0.""" + pr_data = [ + { + "number": 10, + "url": "https://github.com/acme/proj/pull/10", + "headRepositoryOwner": {"login": "other-user"}, + }, + ] + mock_run.return_value = subprocess.CompletedProcess( + [], 0, json.dumps(pr_data), "", + ) + code = publish.main([ + "check-existing", "--repo", "acme/proj", + "--head", "jsmith:feat/x", + ]) + self.assertEqual(code, publish.EXIT_SUCCESS) + + @mock.patch.object(publish, "run") + def test_gitlab_existing_mr_exits_5(self, mock_run: mock.Mock) -> None: + mr_data = [{"iid": 7, "web_url": "https://gitlab.com/grp/proj/-/merge_requests/7"}] + mock_run.return_value = subprocess.CompletedProcess( + [], 0, json.dumps(mr_data), "", + ) + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + code = publish.main([ + "check-existing", "--repo", "grp/proj", + "--head", "feat/x", "--platform", "gitlab", + ]) + self.assertEqual(code, publish.EXIT_EXISTING_FOUND) + + @mock.patch.object(publish, "run") + def test_gitlab_fork_filter_by_project_id(self, mock_run: mock.Mock) -> None: + """Fork-aware GitLab: filters by source_project_id.""" + mr_data = [ + {"iid": 1, "source_project_id": 100}, + {"iid": 2, "source_project_id": 200}, + ] + mock_run.side_effect = [ + # glab mr list + subprocess.CompletedProcess([], 0, json.dumps(mr_data), ""), + # glab api projects/... + subprocess.CompletedProcess([], 0, "200\n", ""), + ] + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + code = publish.main([ + "check-existing", "--repo", "grp/proj", + "--head", "jsmith/proj:feat/x", "--platform", "gitlab", + ]) + self.assertEqual(code, publish.EXIT_EXISTING_FOUND) + result = json.loads(buf.getvalue()) + self.assertEqual(result["iid"], 2) + + +# --------------------------------------------------------------------------- +# JSON encoding edge case tests +# --------------------------------------------------------------------------- + + +class TestJSONEncoding(unittest.TestCase): + """Verify json.dumps handles the edge cases that motivated the rewrite.""" + + def test_newlines_roundtrip(self) -> None: + data = {"body": "line1\nline2\nline3"} + raw = json.dumps(data) + parsed = json.loads(raw) + self.assertEqual(parsed["body"], "line1\nline2\nline3") + + def test_quotes_roundtrip(self) -> None: + data = {"msg": 'He said "hello"'} + raw = json.dumps(data) + parsed = json.loads(raw) + self.assertEqual(parsed["msg"], 'He said "hello"') + + def test_backslash_roundtrip(self) -> None: + data = {"path": "C:\\Users\\test"} + raw = json.dumps(data) + parsed = json.loads(raw) + self.assertEqual(parsed["path"], "C:\\Users\\test") + + def test_control_chars_roundtrip(self) -> None: + data = {"tab": "a\tb", "cr": "x\ry", "null": "a\x00b"} + raw = json.dumps(data) + parsed = json.loads(raw) + self.assertEqual(parsed["tab"], "a\tb") + self.assertEqual(parsed["cr"], "x\ry") + self.assertEqual(parsed["null"], "a\x00b") + + def test_leading_zero_string_preserved(self) -> None: + """The whole point: '007' stays '007', not 7.""" + data = {"pr_number": "007"} + raw = json.dumps(data) + parsed = json.loads(raw) + self.assertIsInstance(parsed["pr_number"], str) + self.assertEqual(parsed["pr_number"], "007") + + def test_unicode_preserved(self) -> None: + data = {"emoji": "\U0001f600", "accent": "café"} + raw = json.dumps(data, ensure_ascii=False) + parsed = json.loads(raw) + self.assertEqual(parsed["emoji"], "\U0001f600") + self.assertEqual(parsed["accent"], "café") + + def test_empty_string_value(self) -> None: + data = {"empty": ""} + raw = json.dumps(data) + parsed = json.loads(raw) + self.assertEqual(parsed["empty"], "") + + def test_mixed_special_chars(self) -> None: + """Combo: newlines + quotes + backslashes in one value.""" + val = 'line1\n"quoted"\npath\\to\\file' + data = {"complex": val} + raw = json.dumps(data) + parsed = json.loads(raw) + self.assertEqual(parsed["complex"], val) + + +# --------------------------------------------------------------------------- +# create-pr / create-mr argument validation +# --------------------------------------------------------------------------- + + +class TestCreatePRValidation(unittest.TestCase): + """Verify create-pr argument validation.""" + + def test_body_file_not_found(self) -> None: + with self.assertRaises(SystemExit) as ctx: + publish.main([ + "create-pr", "--base", "main", "--head", "feat/x", + "--title", "Fix", "--body-file", "/nonexistent/file.md", + ]) + self.assertEqual(ctx.exception.code, publish.EXIT_ARG_ERROR) + + @mock.patch.object(publish, "run") + def test_body_file_used(self, mock_run: mock.Mock) -> None: + mock_run.return_value = subprocess.CompletedProcess( + [], 0, "https://github.com/acme/proj/pull/1\n", "", + ) + with tempfile.NamedTemporaryFile(mode="w", suffix=".md", delete=False) as f: + f.write("PR body content") + f.flush() + try: + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + code = publish.main([ + "create-pr", "--base", "main", "--head", "feat/x", + "--title", "Fix", "--body-file", f.name, + ]) + self.assertEqual(code, 0) + # Verify --body-file was in the command + call_args = mock_run.call_args[0][0] + self.assertIn("--body-file", call_args) + self.assertIn(f.name, call_args) + finally: + os.unlink(f.name) + + @mock.patch.object(publish, "run") + def test_inline_body_used(self, mock_run: mock.Mock) -> None: + mock_run.return_value = subprocess.CompletedProcess( + [], 0, "https://github.com/acme/proj/pull/1\n", "", + ) + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + publish.main([ + "create-pr", "--base", "main", "--head", "feat/x", + "--title", "Fix", "--body", "Inline body text", + ]) + call_args = mock_run.call_args[0][0] + self.assertIn("--body", call_args) + idx = call_args.index("--body") + self.assertEqual(call_args[idx + 1], "Inline body text") + + @mock.patch.object(publish, "run") + def test_empty_body_default(self, mock_run: mock.Mock) -> None: + mock_run.return_value = subprocess.CompletedProcess( + [], 0, "https://github.com/acme/proj/pull/1\n", "", + ) + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + publish.main([ + "create-pr", "--base", "main", "--head", "feat/x", + "--title", "Fix", + ]) + call_args = mock_run.call_args[0][0] + self.assertIn("--body", call_args) + idx = call_args.index("--body") + self.assertEqual(call_args[idx + 1], "") + + @mock.patch.object(publish, "run") + def test_labels_passed(self, mock_run: mock.Mock) -> None: + mock_run.return_value = subprocess.CompletedProcess( + [], 0, "https://github.com/acme/proj/pull/1\n", "", + ) + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + publish.main([ + "create-pr", "--base", "main", "--head", "feat/x", + "--title", "Fix", "--labels", "bug,urgent", + ]) + call_args = mock_run.call_args[0][0] + self.assertIn("--label", call_args) + idx = call_args.index("--label") + self.assertEqual(call_args[idx + 1], "bug,urgent") + + +class TestCreateMRValidation(unittest.TestCase): + """Verify create-mr argument validation.""" + + def test_desc_file_not_found(self) -> None: + with self.assertRaises(SystemExit) as ctx: + publish.main([ + "create-mr", "--source", "x", "--target", "main", + "--title", "T", "--desc-file", "/nonexistent/file.md", + ]) + self.assertEqual(ctx.exception.code, publish.EXIT_ARG_ERROR) + + @mock.patch.object(publish, "run") + def test_desc_file_read(self, mock_run: mock.Mock) -> None: + mock_run.return_value = subprocess.CompletedProcess( + [], 0, "https://gitlab.com/grp/proj/-/merge_requests/1\n", "", + ) + with tempfile.NamedTemporaryFile(mode="w", suffix=".md", delete=False) as f: + f.write("MR description from file") + f.flush() + try: + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + publish.main([ + "create-mr", "--source", "docs/fix", + "--target", "main", "--title", "Docs update", + "--desc-file", f.name, + ]) + call_args = mock_run.call_args[0][0] + self.assertIn("--description", call_args) + idx = call_args.index("--description") + self.assertEqual(call_args[idx + 1], "MR description from file") + finally: + os.unlink(f.name) + + +# --------------------------------------------------------------------------- +# Push tests (with subprocess mocking) +# --------------------------------------------------------------------------- + + +class TestPush(unittest.TestCase): + """Verify push behavior.""" + + @mock.patch.object(publish, "run") + def test_push_success(self, mock_run: mock.Mock) -> None: + mock_run.side_effect = [ + # git remote get-url -> success + subprocess.CompletedProcess([], 0, "https://github.com/user/repo.git\n", ""), + # git push -u -> success + subprocess.CompletedProcess([], 0, "", ""), + ] + code = publish.main(["push", "--remote", "fork", "--branch", "feat/x"]) + self.assertEqual(code, 0) + + @mock.patch.object(publish, "run") + def test_push_failure_exits_3(self, mock_run: mock.Mock) -> None: + mock_run.side_effect = [ + # git remote get-url -> success + subprocess.CompletedProcess([], 0, "https://github.com/user/repo.git\n", ""), + # git push -u -> failure + subprocess.CompletedProcess([], 128, "", "permission denied"), + ] + with self.assertRaises(SystemExit) as ctx: + publish.main(["push", "--remote", "fork", "--branch", "feat/x"]) + self.assertEqual(ctx.exception.code, publish.EXIT_PUSH_FAIL) + + +if __name__ == "__main__": + unittest.main() diff --git a/bugfix/SKILL.md b/bugfix/SKILL.md index 20ea747d..5a86db93 100644 --- a/bugfix/SKILL.md +++ b/bugfix/SKILL.md @@ -1,6 +1,6 @@ --- name: bugfix -version: 0.8.0 +version: 0.8.1 description: >- Diagnostic and repair workflow that analyzes error logs, traces root causes, implements fixes, and verifies with regression tests. diff --git a/bugfix/skills/pr.md b/bugfix/skills/pr.md index 88981529..013de8f3 100644 --- a/bugfix/skills/pr.md +++ b/bugfix/skills/pr.md @@ -35,6 +35,18 @@ the documented recovery paths instead of guessing. - **Never attempt `gh repo fork` without asking the user first.** - **Never fall back to patch files without exhausting all other options.** +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.py +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-pr`, and `save-metadata`. See the script header for full usage. + ## Process ### Placeholders Used in This Skill @@ -68,35 +80,40 @@ commands run from there. If the user provides a path or the repo is obvious from session context (prior commands, artifacts), use that directly. +Now that you are inside the project repo, resolve the shared script to an +absolute path so it remains valid regardless of working directory: + +```bash +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.py" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands. + ### Step 1: Pre-flight Checks Run ALL of these before doing anything else. Do not skip any. -**1a. Check GitHub CLI authentication and determine GH_USER:** +**1a. Run the shared pre-flight checks:** ```bash -gh auth status +python3 "$PUBLISH_SCRIPT" preflight --platform github ``` -- If authenticated, determine `GH_USER` — the **real user's** GitHub username - (not the bot). Try these in order: - -```bash -# Works for normal user tokens: -gh api user --jq .login 2>/dev/null - -# If that fails (403), you're running as a GitHub App/bot. -# Get the real user from the app installation: -gh api /installation/repositories --jq '.repositories[0].owner.login' -``` +Parse the structured output: +- `auth_ok` — whether `gh auth` succeeded +- `auth_user` — the GitHub username (`GH_USER`) +- `branch` — current branch name +- `has_uncommitted` / `has_staged` / `has_untracked` — whether there are uncommitted, staged, or untracked changes -The `/installation/repositories` endpoint works because GitHub Apps are -installed on user accounts — the repo owner is the actual user. +If `auth_ok=true`, set `GH_USER` from `auth_user`. If `auth_user` is +empty (GitHub App/bot), the script already tried the +`/installation/repositories` fallback. -- If not authenticated: note this — several later steps depend on `gh`. But - do NOT dump all manual instructions yet. Continue the remaining pre-flight - checks (1b–1e) to gather as much information as possible from git alone. - After pre-flight, you will present options to the user. +If `auth_ok=false`: note this — several later steps depend on `gh`. But +do NOT dump all manual instructions yet. Continue the remaining pre-flight +checks (1b–1d) to gather as much information as possible from git alone. +After pre-flight, you will present options to the user. **1b. Check git configuration:** @@ -148,15 +165,11 @@ git remote get-url origin | sed -E 's#.*/([^/]+/[^/]+?)(\.git)?$#\1#' Record the result as `UPSTREAM_OWNER/REPO` — you'll need it later. -**1e. Check current branch and changes:** - -```bash -git status -git diff --stat -``` - -Confirm there are actual changes to commit. If there are no changes, stop -and tell the user. +Confirm there are actual changes to commit (from the pre-flight output's +`has_uncommitted`, `has_staged`, or `has_untracked` fields). If all three +are `false`, there are no changes — stop and tell the user. If +`has_untracked` is `true`, warn the user about untracked files and ask +whether they should be included in the commit. **Pre-flight summary:** Before moving on, you should now know: `UPSTREAM_OWNER/REPO`, which remotes exist, and whether there are changes to @@ -440,10 +453,10 @@ to write an accurate commit message. Don't make up details. ### Step 8: Push to Fork ```bash -git push -u fork bugfix/BRANCH_NAME +python3 "$PUBLISH_SCRIPT" push --remote fork --branch bugfix/BRANCH_NAME ``` -**If this fails:** +**If the script exits with code 3 (push failed):** - **Authentication error**: Check `gh auth status` again. The user may need to re-authenticate or the sandbox may be blocking network access. @@ -456,78 +469,60 @@ access. Please run: `git push -u fork BRANCH_NAME`" ### Step 9: Create the Draft PR -**If a pull request already exists** for this branch on -`UPSTREAM_OWNER/REPO`, skip this step and proceed to **Confirm and -Report**. Check with: +**Check for an existing PR** before attempting creation. Use +`FORK_OWNER:bugfix/BRANCH_NAME` so the check matches only PRs from +this fork (plain `bugfix/BRANCH_NAME` would match any fork's branch +with the same name): ```bash -gh pr list --repo UPSTREAM_OWNER/REPO --head bugfix/BRANCH_NAME --json number,url --jq '.[0] // empty' +python3 "$PUBLISH_SCRIPT" check-existing \ + --repo UPSTREAM_OWNER/REPO \ + --head FORK_OWNER:bugfix/BRANCH_NAME ``` -If the command fails (auth error, network error, API error), **stop and -report the failure** — do not fall through to PR creation. Only proceed -when the command succeeds: a result means the PR already exists (skip to -Step 10 and report its URL); an empty result means no existing PR (continue -with creation below). +If exit code is 5, a PR already exists — skip to Step 10 and report its +URL. If the command fails (auth error, network error, API error), **stop +and report the failure** — do not fall through to PR creation. **PR title format:** Use **`[ISSUE_KEY]: short description in lowercase`**. If the artifact `.artifacts/bugfix/{issue}/pr-description.md` exists and has a `## Title` line in this format, use that title. Otherwise set `ISSUE_KEY` from the branch name or context (e.g. Jira EDM-1234, GitHub #47) and build the title as `[ISSUE_KEY]: short description`. -**Try `gh pr create` first** (it works for normal user tokens): +**Create the PR using the shared script** (works for normal user tokens): + +If the `--body-file` artifact exists: ```bash -gh pr create \ - --draft \ +python3 "$PUBLISH_SCRIPT" create-pr \ --repo UPSTREAM_OWNER/REPO \ --head FORK_OWNER:bugfix/BRANCH_NAME \ --base main \ --title "[ISSUE_KEY]: short description in lowercase" \ - --body-file .artifacts/bugfix/{issue}/pr-description.md + --body-file .artifacts/bugfix/{issue}/pr-description.md \ + --draft ``` -**Key flags explained:** - -- `--repo`: The upstream repository (where the PR goes). REQUIRED for cross-fork PRs. -- `--head`: Must be `FORK_OWNER:BRANCH_NAME` format for fork-based PRs. Without the - owner prefix, GitHub looks for the branch on the upstream repo and fails. -- `--base`: The target branch on upstream (usually `main`). -- `--draft`: Always submit as draft first. -- `--title`: PR title must be `[ISSUE_KEY]: short description`. Prefer the title from the artifact's `## Title` section if present. -- `--body-file`: Use the PR description artifact if `/document` was run. - -**If `--body-file` artifact doesn't exist**, use `--body` with inline content: +If the artifact doesn't exist, generate the PR body inline (AI-dependent — +see the template in this skill's Notes section) and pass it with `--body`: ```bash -gh pr create \ - --draft \ +python3 "$PUBLISH_SCRIPT" create-pr \ --repo UPSTREAM_OWNER/REPO \ --head FORK_OWNER:bugfix/BRANCH_NAME \ --base main \ --title "[ISSUE_KEY]: short description in lowercase" \ - --body "## Problem -WHAT_WAS_BROKEN - -## Root Cause -WHY_IT_WAS_BROKEN - -## Fix -WHAT_THIS_PR_CHANGES - -## Testing -HOW_THE_FIX_WAS_VERIFIED - -## Confidence -HIGH_MEDIUM_LOW — BRIEF_JUSTIFICATION - -## Rollback -HOW_TO_REVERT_IF_SOMETHING_GOES_WRONG + --body "PR_BODY_TEXT" \ + --draft +``` -## Risk Assessment -LOW_MEDIUM_HIGH — WHAT_COULD_BE_AFFECTED +**Key flags explained:** -Fixes #ISSUE_NUMBER" -``` +- `--repo`: The upstream repository (where the PR goes). REQUIRED for cross-fork PRs. +- `--head`: Must be `FORK_OWNER:BRANCH_NAME` format for fork-based PRs. Without the + owner prefix, GitHub looks for the branch on the upstream repo and fails. +- `--base`: The target branch on upstream (usually `main`). +- `--draft`: Always submit as draft first. +- `--title`: PR title must be `[ISSUE_KEY]: short description`. Prefer the title from the artifact's `## Title` section if present. -**If `gh pr create` fails (403, "Resource not accessible by integration", etc.):** +**If the script exits with code 4 (PR creation failed, e.g., 403, "Resource not accessible by integration"):** This is the expected outcome when running as a GitHub App bot. Do NOT retry, do NOT debug further, do NOT fall back to a patch file. Instead: diff --git a/design/SKILL.md b/design/SKILL.md index b060cc28..d71022eb 100644 --- a/design/SKILL.md +++ b/design/SKILL.md @@ -1,6 +1,6 @@ --- name: design -version: 0.10.0 +version: 0.10.1 description: >- Design-and-decompose workflow that takes a PRD, researches the problem space, drafts a technical design document with a requirement-anchored testplan, diff --git a/design/skills/publish.md b/design/skills/publish.md index fa50aa1b..adf44a43 100644 --- a/design/skills/publish.md +++ b/design/skills/publish.md @@ -21,8 +21,32 @@ the user before taking action. - **No force-push.** No destructive git operations. - **No direct commits to main.** Always use a feature branch. +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.py +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-pr`, and `save-metadata`. See the script header for full usage. + ## Process +### Prerequisites: Resolve Script Path + +Before any `cd` or subshell that changes the working directory, resolve +the shared script to an absolute path so it remains valid: + +```bash +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.py" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands. + ### Step 1: Read the Design Document Read `.artifacts/design/{issue-key}/03-design.md`. @@ -56,13 +80,13 @@ validated `docs_repo_path` and `docs_repo_remote`. ### Step 3: Pre-Flight Checks -Verify the environment: +Run the shared pre-flight checks from the docs repo directory: ```bash -gh auth status +(cd "{docs_repo_path}" && python3 "$PUBLISH_SCRIPT" preflight --platform github) ``` -In the docs repo directory: +Parse the output to confirm `auth_ok=true`. Also verify the docs repo state: ```bash git -C "{docs_repo_path}" remote -v @@ -206,12 +230,14 @@ git -C "{docs_repo_path}" commit -m "Add design document and testplan for {issue ### Step 5: Push and Create PR +Push the branch using the shared script (run from the docs repo): + ```bash -git -C "{docs_repo_path}" push -u origin {branch-name} +(cd "{docs_repo_path}" && python3 "$PUBLISH_SCRIPT" push --remote origin --branch {branch-name}) ``` Read the design document and identify specific areas that warrant reviewer -attention: +attention (AI-dependent): - Open questions from Section 9 (list each by title) - Sections with remaining TBD markers - Key architectural decisions that have significant trade-offs @@ -249,37 +275,54 @@ draft PR. Set `{pr-title}` based on whether `{issue-key}` is a Jira key: if yes, use `{issue-key}: Design - {title}`; otherwise use `Design: {title}`. +First, check whether a PR already exists for this branch: + ```bash -gh pr create --draft --repo {owner}/{repo} --base {base-branch} --head {branch-name} --title "{pr-title}" --body-file .artifacts/design/{issue-key}/08-pr-description.md +python3 "$PUBLISH_SCRIPT" check-existing --repo {owner}/{repo} --head {branch-name} ``` -### Step 6: Save Publish Metadata +If exit code is 5, a PR already exists — skip to Step 6 and report its +URL. Parse the PR number from the returned JSON. If the command fails +(non-zero exit other than 5), stop and report the error. If exit code +is 0, create a new PR: -Write `.artifacts/design/{issue-key}/publish-metadata.json`: +```bash +python3 "$PUBLISH_SCRIPT" create-pr \ + --repo {owner}/{repo} \ + --base {base-branch} \ + --head {branch-name} \ + --title "{pr-title}" \ + --body-file .artifacts/design/{issue-key}/08-pr-description.md \ + --draft +``` + +The script prints the PR URL on stdout. Parse the PR number from the URL path. + +### Step 6: Save Publish Metadata If `04-testplan.md` was published: -```json -{ - "release": "{release}", - "feature": "{feature}", - "design_file_path": "{release}/{feature}/design.md", - "testplan_file_path": "{release}/{feature}/testplan.md", - "pr_number": {pr-number}, - "branch": "{branch-name}" -} +```bash +python3 "$PUBLISH_SCRIPT" save-metadata \ + --file .artifacts/design/{issue-key}/publish-metadata.json \ + release={release} \ + feature={feature} \ + design_file_path={release}/{feature}/design.md \ + testplan_file_path={release}/{feature}/testplan.md \ + pr_number={pr-number} \ + branch={branch-name} ``` -If no testplan was published, omit `testplan_file_path` entirely: +If no testplan was published, omit `testplan_file_path`: -```json -{ - "release": "{release}", - "feature": "{feature}", - "design_file_path": "{release}/{feature}/design.md", - "pr_number": {pr-number}, - "branch": "{branch-name}" -} +```bash +python3 "$PUBLISH_SCRIPT" save-metadata \ + --file .artifacts/design/{issue-key}/publish-metadata.json \ + release={release} \ + feature={feature} \ + design_file_path={release}/{feature}/design.md \ + pr_number={pr-number} \ + branch={branch-name} ``` ### Step 7: Report to User diff --git a/docs-writer/SKILL.md b/docs-writer/SKILL.md index b4ca9873..de25ef2a 100644 --- a/docs-writer/SKILL.md +++ b/docs-writer/SKILL.md @@ -1,6 +1,6 @@ --- name: docs-writer -version: 0.3.1 +version: 0.3.2 description: Documentation workflow that converts requirements into structured AsciiDoc sections, runs Vale for style compliance, and produces merge-ready content. Use when creating or updating AsciiDoc documentation from Jira tickets, GitHub issues, or feature descriptions. --- # Docs Writer Workflow Orchestrator diff --git a/docs-writer/skills/create-mr.md b/docs-writer/skills/create-mr.md index dce6262e..cf49a87a 100644 --- a/docs-writer/skills/create-mr.md +++ b/docs-writer/skills/create-mr.md @@ -29,6 +29,32 @@ recovery paths instead of guessing. - **Always create a draft MR.** Let the author mark it ready after review. - **Never attempt `glab repo fork` without asking the user first.** +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.py +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-mr`, and `save-metadata`. For GitLab workflows, pass +`--platform gitlab` to `preflight` and `check-existing`. See the script +header for full usage. + +### Prerequisites: Resolve Script Path + +Before running any subcommands, resolve the shared script to an +absolute path so it remains valid regardless of working directory: + +```bash +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.py" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands. + ## Process ### Placeholders Used in This Skill @@ -41,26 +67,30 @@ These are determined during pre-flight checks. Record each value as you go. | `UPSTREAM_PROJECT` | Step 1d: project path from remote URL | `red-hat-enterprise-openshift-documentation/edge-manager` | | `FORK_PROJECT` | Step 2: user's fork path | `jsmith/edge-manager` | | `BRANCH_NAME` | Step 4: the branch you create | `docs/RHEM-456-enrollment-api` | +| `PUSH_REMOTE` | Step 2/3: remote name to push to | `origin` or `fork` | | `TICKET_ID` | From artifacts directory or user input | `RHEM-456` | ### Step 1: Pre-flight Checks Run ALL of these before doing anything else. Do not skip any. -**1a. Check GitLab CLI authentication and determine GL_USER:** +**1a. Run the shared pre-flight checks:** ```bash -glab auth status +python3 "$PUBLISH_SCRIPT" preflight --platform gitlab ``` -- If authenticated, determine `GL_USER`: +Parse the structured output: +- `auth_ok` — whether `glab auth` succeeded +- `auth_user` — the GitLab username (`GL_USER`) +- `branch` — current branch name +- `has_uncommitted` / `has_staged` / `has_untracked` — whether there are uncommitted, staged, or untracked changes -```bash -glab api user --jq .username -``` +If `auth_ok=true`, set `GL_USER` from `auth_user`. -- If not authenticated: note this and continue the remaining pre-flight checks (1b–1e) to gather as much information as possible from git alone. After pre-flight, present options - to the user. +If `auth_ok=false`: note this and continue the remaining pre-flight checks +(1b–1d) to gather as much information as possible from git alone. After +pre-flight, present options to the user. **1b. Check git configuration:** @@ -111,14 +141,11 @@ git remote get-url origin | sed -E 's#.*[:/]([^/]+/[^/]+?)(\.git)?$#\1#' Record the result as `UPSTREAM_PROJECT`. -**1e. Check current branch and changes:** - -```bash -git status -git diff --stat -``` - -Confirm there are actual changes to commit. If there are no changes, stop and tell the user. +Confirm there are actual changes to commit (from the pre-flight output's +`has_uncommitted`, `has_staged`, or `has_untracked` fields). If all three +are `false`, there are no changes — stop and tell the user. If +`has_untracked` is `true`, warn the user about untracked files and ask +whether they should be included in the commit. **Pre-flight summary:** Before moving on, you should now know: `UPSTREAM_PROJECT`, which remotes exist, and whether there are changes to commit. You may also know `GL_USER` (if auth is available). @@ -234,19 +261,19 @@ Don't make up details. ### Step 6: Push -**Direct push (write access):** - -```bash -git push -u origin docs/BRANCH_NAME -``` - -**Fork push:** +Use the remote identified during Step 2 (direct push) or Step 3 (fork +workflow) as `PUSH_REMOTE`. Set `PUSH_REMOTE` to the actual remote name +discovered from `git remote -v` — typically `origin` for direct push or +`fork` for fork-based workflows: ```bash -git push -u fork docs/BRANCH_NAME +# Set PUSH_REMOTE based on the push strategy determined in Step 2/3: +# - Direct push: PUSH_REMOTE is the remote pointing to UPSTREAM_PROJECT +# - Fork workflow: PUSH_REMOTE is the remote pointing to FORK_PROJECT +python3 "$PUBLISH_SCRIPT" push --remote "$PUSH_REMOTE" --branch "docs/$BRANCH_NAME" ``` -**If push fails:** +**If the script exits with code 3 (push failed):** - **Authentication error**: Check `glab auth status`. User may need to re-authenticate. - **Permission denied**: Verify the remote URL points to the correct project. @@ -256,36 +283,52 @@ git push -u fork docs/BRANCH_NAME **MR title format:** Use `[TICKET_ID]: short description in lowercase`. +**Building the description:** Use the MR description prepared by the `/apply` +phase at `.artifacts/${ticket_id}/04-mr-description.md`. If the file does not +exist, build the description (AI-dependent) from the context artifact +(`01-context.md`) and plan artifact (`02-plan.md`). + +**Check for an existing MR** before attempting creation: + +```bash +# Direct push: +python3 "$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "docs/$BRANCH_NAME" --platform gitlab + +# Fork workflow (project:branch filters by source project to avoid cross-fork false matches): +python3 "$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "FORK_PROJECT:docs/$BRANCH_NAME" --platform gitlab +``` + +If exit code is 5, an MR already exists — skip to Step 8 and report its +URL. If the command fails (non-zero exit other than 5), stop and report +the error. If exit code is 0, create a new MR: + **Direct push (user has write access):** ```bash -glab mr create \ - --draft \ - --source-branch docs/BRANCH_NAME \ - --target-branch main \ +python3 "$PUBLISH_SCRIPT" create-mr \ + --source "docs/$BRANCH_NAME" \ + --target main \ --title "[TICKET_ID]: short description" \ - --description "DESCRIPTION" \ - --yes + --desc-file ".artifacts/${ticket_id}/04-mr-description.md" \ + --draft ``` +If no description file exists, use `--description` with inline text instead. + **Fork workflow:** ```bash -glab mr create \ - --draft \ - --repo UPSTREAM_PROJECT \ +python3 "$PUBLISH_SCRIPT" create-mr \ + --project UPSTREAM_PROJECT \ --head FORK_PROJECT \ - --source-branch docs/BRANCH_NAME \ - --target-branch main \ + --source "docs/$BRANCH_NAME" \ + --target main \ --title "[TICKET_ID]: short description" \ - --description "DESCRIPTION" \ - --yes + --desc-file ".artifacts/${ticket_id}/04-mr-description.md" \ + --draft ``` -**Building the description:** Use the MR description prepared by the `/apply` phase at `.artifacts/${ticket_id}/04-mr-description.md`. If the file does not exist, build the -description from the context artifact (`01-context.md`) and plan artifact (`02-plan.md`). - -**If `glab mr create` fails:** +**If the script exits with code 4 (MR creation failed):** 1. **Write the MR description** to `.artifacts/${ticket_id}/04-mr-description.md` diff --git a/e2e/SKILL.md b/e2e/SKILL.md index 3fd17b10..6003deda 100644 --- a/e2e/SKILL.md +++ b/e2e/SKILL.md @@ -1,6 +1,6 @@ --- name: e2e -version: 0.7.0 +version: 0.7.1 description: >- Story-to-e2e-test workflow that takes a Jira [QE] Story, discovers the project's e2e testing infrastructure, plans test scenarios, writes e2e diff --git a/e2e/skills/publish.md b/e2e/skills/publish.md index 976501e0..becf5ad0 100644 --- a/e2e/skills/publish.md +++ b/e2e/skills/publish.md @@ -23,8 +23,32 @@ user before taking action. - **No direct commits to main.** The feature branch must already exist from `/code`. - **Validation must have passed.** Check for a passing validation report before proceeding. +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.py +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-pr`, and `save-metadata`. See the script header for full usage. + ## Process +### Prerequisites: Resolve Script Path + +Before running any subcommands, resolve the shared script to an +absolute path so it remains valid regardless of working directory: + +```bash +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.py" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands. + ### Step 1: Pre-Flight Checks Verify readiness: @@ -48,19 +72,18 @@ Verify readiness: If there are no commits ahead of the Local Base, there's nothing to publish. -3. Check for uncommitted changes: +3. Run the shared pre-flight checks: ```bash - git status + python3 "$PUBLISH_SCRIPT" preflight --platform github ``` - If there are uncommitted changes, ask the user how to proceed. - -4. Verify GitHub CLI is authenticated: - - ```bash - gh auth status - ``` + Parse the output and check: + - `auth_ok` — if `false`, **stop and tell the user** that GitHub CLI + authentication is required to push and create a PR. Suggest running + `gh auth login` and retrying `/publish`. Do not continue without auth. + - `has_uncommitted`, `has_staged`, `has_untracked` — if any are `true`, + ask the user how to proceed (commit, stash, or include untracked files). ### Step 2: Cross-Cutting Review @@ -115,7 +138,7 @@ Confirm with the user before proceeding. ### Step 4: Push Branch ```bash -git push -u origin {branch-name} +python3 "$PUBLISH_SCRIPT" push --remote origin --branch {branch-name} ``` ### Step 5: Create PR Description @@ -164,11 +187,36 @@ In either case, save the result to Check the **Repository Topology** section of `01-context.md` to determine whether this is a fork-based workflow. +First, check whether a PR already exists for this branch: + +For fork-based workflows, use `{fork-owner}:{branch-name}` as the +`--head` value so the check matches only PRs from this fork (plain +`{branch-name}` would match any fork's branch with the same name): + +```bash +# Fork-based: +python3 "$PUBLISH_SCRIPT" check-existing --repo {upstream-owner}/{repo} --head {fork-owner}:{branch-name} + +# Direct clone: +python3 "$PUBLISH_SCRIPT" check-existing --repo {upstream-owner}/{repo} --head {branch-name} +``` + +If exit code is 5, a PR already exists — parse the PR number and URL +from the returned JSON output, then skip to Step 7 and use those values +in the metadata. If the command fails (non-zero exit other than 5), +stop and report the error. If exit code is 0, create a new PR. + **If the repo is a fork** (Origin is `{fork-owner}/{repo}`, Upstream is `{upstream-owner}/{repo}`): ```bash -gh pr create --draft --repo {upstream-owner}/{repo} --base {pr-target} --head {fork-owner}:{branch-name} --title "{issue-key}: {story title}" --body-file .artifacts/e2e/{issue-key}/06-pr-description.md +python3 "$PUBLISH_SCRIPT" create-pr \ + --repo {upstream-owner}/{repo} \ + --base {pr-target} \ + --head {fork-owner}:{branch-name} \ + --title "{issue-key}: {story title}" \ + --body-file .artifacts/e2e/{issue-key}/06-pr-description.md \ + --draft ``` The `--repo` flag targets the upstream repository (where the PR lives), @@ -178,12 +226,22 @@ branch (on the fork). **If the repo is a direct clone** (not a fork): ```bash -gh pr create --draft --base {pr-target} --head {branch-name} --title "{issue-key}: {story title}" --body-file .artifacts/e2e/{issue-key}/06-pr-description.md +python3 "$PUBLISH_SCRIPT" create-pr \ + --base {pr-target} \ + --head {branch-name} \ + --title "{issue-key}: {story title}" \ + --body-file .artifacts/e2e/{issue-key}/06-pr-description.md \ + --draft ``` -Parse the PR number and URL from the `gh pr create` output. The command -prints a URL like `https://github.com/owner/repo/pull/42` — extract the -number from the URL path. +The script prints the PR URL on stdout. Parse the PR number from the URL +path (e.g., `https://github.com/owner/repo/pull/42` → `42`). + +If the script exits with code 4 (PR creation failed), fall back to +providing the user with a GitHub compare URL: + +- Fork-based: `https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{fork-owner}:{branch-name}?expand=1` +- Direct clone: `https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{branch-name}?expand=1` ### Step 7: Save Publish Metadata @@ -191,37 +249,35 @@ Read `{owner}/{repo}` from the **Origin** field of the Repository Topology section of `01-context.md`. If the repo is a fork, also read the **Upstream** field. -Write `.artifacts/e2e/{issue-key}/publish-metadata.json`. - The `repo` field always refers to where the PR lives. The `origin` field records the repo that was pushed to. **If the repo is a fork** (set `repo` to the upstream, `origin` to the fork): -```json -{ - "repo": "{upstream-owner}/{repo}", - "origin": "{fork-owner}/{repo}", - "branch": "{branch-name}", - "base": "{pr-target}", - "pr_number": {pr-number}, - "pr_url": "{url from gh pr create output}", - "jira_key": "{issue-key}" -} +```bash +python3 "$PUBLISH_SCRIPT" save-metadata \ + --file .artifacts/e2e/{issue-key}/publish-metadata.json \ + repo={upstream-owner}/{repo} \ + origin={fork-owner}/{repo} \ + branch={branch-name} \ + base={pr-target} \ + pr_number={pr-number} \ + pr_url={url-from-create-pr-output} \ + jira_key={issue-key} ``` **If the repo is a direct clone** (`repo` and `origin` are the same): -```json -{ - "repo": "{owner}/{repo}", - "origin": "{owner}/{repo}", - "branch": "{branch-name}", - "base": "{pr-target}", - "pr_number": {pr-number}, - "pr_url": "{url from gh pr create output}", - "jira_key": "{issue-key}" -} +```bash +python3 "$PUBLISH_SCRIPT" save-metadata \ + --file .artifacts/e2e/{issue-key}/publish-metadata.json \ + repo={owner}/{repo} \ + origin={owner}/{repo} \ + branch={branch-name} \ + base={pr-target} \ + pr_number={pr-number} \ + pr_url={url-from-create-pr-output} \ + jira_key={issue-key} ``` ### Step 8: Report to User diff --git a/implement/SKILL.md b/implement/SKILL.md index b9fab8e8..232e590b 100644 --- a/implement/SKILL.md +++ b/implement/SKILL.md @@ -1,6 +1,6 @@ --- name: implement -version: 0.9.0 +version: 0.9.1 description: >- Story-to-code workflow that takes a Jira Story, plans the implementation, writes contract-based tests and production code via TDD, validates against diff --git a/implement/skills/publish.md b/implement/skills/publish.md index b58d3e2b..27c0902e 100644 --- a/implement/skills/publish.md +++ b/implement/skills/publish.md @@ -23,8 +23,32 @@ user before taking action. - **No direct commits to main.** The feature branch must already exist from `/code`. - **Validation must have passed.** Check for a passing validation report before proceeding. +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.py +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-pr`, and `save-metadata`. See the script header for full usage. + ## Process +### Prerequisites: Resolve Script Path + +Before running any subcommands, resolve the shared script to an +absolute path so it remains valid regardless of working directory: + +```bash +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.py" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands. + ### Step 1: Pre-Flight Checks Verify readiness: @@ -48,19 +72,15 @@ Verify readiness: If there are no commits ahead of the Local Base, there's nothing to publish. -3. Check for uncommitted changes: +3. Run the shared pre-flight checks: ```bash - git status + python3 "$PUBLISH_SCRIPT" preflight --platform github ``` - If there are uncommitted changes, ask the user how to proceed. - -4. Verify GitHub CLI is authenticated: - - ```bash - gh auth status - ``` + Parse the output to confirm `auth_ok=true` and check for + `has_uncommitted=true` or `has_staged=true`. If there are uncommitted + changes, ask the user how to proceed. ### Step 2: Cross-Cutting Review @@ -114,7 +134,7 @@ Confirm with the user before proceeding. ### Step 4: Push Branch ```bash -git push -u origin {branch-name} +python3 "$PUBLISH_SCRIPT" push --remote origin --branch {branch-name} ``` ### Step 5: Create PR Description @@ -160,11 +180,28 @@ In either case, save the result to Check the **Repository Topology** section of `01-context.md` to determine whether this is a fork-based workflow. +First, check whether a PR already exists for this branch: + +```bash +python3 "$PUBLISH_SCRIPT" check-existing --repo {upstream-owner}/{repo} --head {branch-name} +``` + +If exit code is 5, a PR already exists — parse the PR number and URL +from the returned JSON output, then skip to Step 7 and use those values +in the metadata. If the command fails (non-zero exit other than 5), +stop and report the error. If exit code is 0, create a new PR. + **If the repo is a fork** (Origin is `{fork-owner}/{repo}`, Upstream is `{upstream-owner}/{repo}`): ```bash -gh pr create --draft --repo {upstream-owner}/{repo} --base {pr-target} --head {fork-owner}:{branch-name} --title "{issue-key}: {story title}" --body-file .artifacts/implement/{issue-key}/06-pr-description.md +python3 "$PUBLISH_SCRIPT" create-pr \ + --repo {upstream-owner}/{repo} \ + --base {pr-target} \ + --head {fork-owner}:{branch-name} \ + --title "{issue-key}: {story title}" \ + --body-file .artifacts/implement/{issue-key}/06-pr-description.md \ + --draft ``` The `--repo` flag targets the upstream repository (where the PR lives), @@ -174,12 +211,22 @@ branch (on the fork). **If the repo is a direct clone** (not a fork): ```bash -gh pr create --draft --base {pr-target} --head {branch-name} --title "{issue-key}: {story title}" --body-file .artifacts/implement/{issue-key}/06-pr-description.md +python3 "$PUBLISH_SCRIPT" create-pr \ + --base {pr-target} \ + --head {branch-name} \ + --title "{issue-key}: {story title}" \ + --body-file .artifacts/implement/{issue-key}/06-pr-description.md \ + --draft ``` -Parse the PR number and URL from the `gh pr create` output. The command -prints a URL like `https://github.com/owner/repo/pull/42` — extract the -number from the URL path. +The script prints the PR URL on stdout. Parse the PR number from the URL +path (e.g., `https://github.com/owner/repo/pull/42` → `42`). + +If the script exits with code 4 (PR creation failed), fall back to +providing the user with a GitHub compare URL: + +- Fork-based: `https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{fork-owner}:{branch-name}?expand=1` +- Direct clone: `https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{branch-name}?expand=1` ### Step 7: Save Publish Metadata @@ -187,37 +234,35 @@ Read `{owner}/{repo}` from the **Origin** field of the Repository Topology section of `01-context.md`. If the repo is a fork, also read the **Upstream** field. -Write `.artifacts/implement/{issue-key}/publish-metadata.json`. - The `repo` field always refers to where the PR lives. The `origin` field records the repo that was pushed to. **If the repo is a fork** (set `repo` to the upstream, `origin` to the fork): -```json -{ - "repo": "{upstream-owner}/{repo}", - "origin": "{fork-owner}/{repo}", - "branch": "{branch-name}", - "base": "{pr-target}", - "pr_number": {pr-number}, - "pr_url": "{url from gh pr create output}", - "jira_key": "{issue-key}" -} +```bash +python3 "$PUBLISH_SCRIPT" save-metadata \ + --file .artifacts/implement/{issue-key}/publish-metadata.json \ + repo={upstream-owner}/{repo} \ + origin={fork-owner}/{repo} \ + branch={branch-name} \ + base={pr-target} \ + pr_number={pr-number} \ + pr_url={url-from-create-pr-output} \ + jira_key={issue-key} ``` **If the repo is a direct clone** (`repo` and `origin` are the same): -```json -{ - "repo": "{owner}/{repo}", - "origin": "{owner}/{repo}", - "branch": "{branch-name}", - "base": "{pr-target}", - "pr_number": {pr-number}, - "pr_url": "{url from gh pr create output}", - "jira_key": "{issue-key}" -} +```bash +python3 "$PUBLISH_SCRIPT" save-metadata \ + --file .artifacts/implement/{issue-key}/publish-metadata.json \ + repo={owner}/{repo} \ + origin={owner}/{repo} \ + branch={branch-name} \ + base={pr-target} \ + pr_number={pr-number} \ + pr_url={url-from-create-pr-output} \ + jira_key={issue-key} ``` ### Step 8: Report to User diff --git a/prd/SKILL.md b/prd/SKILL.md index 58ba5048..88264564 100644 --- a/prd/SKILL.md +++ b/prd/SKILL.md @@ -1,6 +1,6 @@ --- name: prd -version: 0.10.0 +version: 0.10.1 description: >- Requirements-to-PRD workflow that ingests requirements from Jira, clarifies ambiguities through iterative Q&A, drafts a Product Requirements Document, diff --git a/prd/skills/publish.md b/prd/skills/publish.md index 12237482..01ac3c28 100644 --- a/prd/skills/publish.md +++ b/prd/skills/publish.md @@ -21,8 +21,32 @@ the user before taking action. - **No force-push.** No destructive git operations. - **No direct commits to main.** Always use a feature branch. +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.py +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-pr`, and `save-metadata`. See the script header for full usage. + ## Process +### Prerequisites: Resolve Script Path + +Before any `cd` or subshell that changes the working directory, resolve +the shared script to an absolute path so it remains valid: + +```bash +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.py" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands. + ### Step 1: Read the PRD Read `.artifacts/prd/{issue-key}/03-prd.md`. @@ -56,13 +80,18 @@ validated `docs_repo_path` and `docs_repo_remote`. ### Step 3: Pre-Flight Checks -Verify the environment using the docs repo: +Run the shared pre-flight checks from the docs repo directory: ```bash -gh auth status +(cd "{docs_repo_path}" && python3 "$PUBLISH_SCRIPT" preflight --platform github) ``` -In the docs repo directory: +Parse the output and check `auth_ok`. If `auth_ok=false`, **stop and +tell the user** that GitHub CLI authentication is required to push and +create a PR. Suggest running `gh auth login` and retrying `/publish`. +Do not continue to later steps without authentication. + +If `auth_ok=true`, verify the docs repo state: ```bash git -C "{docs_repo_path}" remote -v @@ -105,6 +134,17 @@ for release and feature slug separately. All git operations in this step run against the **docs repo**, not the source repo. Use `git -C "{docs_repo_path}"` for all commands. +Verify the docs repo has no uncommitted, staged, or untracked changes +before modifying it: + +```bash +git -C "{docs_repo_path}" status --porcelain +``` + +If the output is non-empty, the docs repo has local changes. **Stop and +ask the user** how to proceed — they may need to stash or commit those +changes first. Do not copy files into a dirty working tree. + Check if the branch already exists (locally or on the remote) before creating it: ```bash @@ -175,12 +215,15 @@ git -C "{docs_repo_path}" commit -m "Add PRD for {issue-key}: {title}" ### Step 5: Push and Create PR +Push the branch using the shared script (run from the docs repo): + ```bash -git -C "{docs_repo_path}" push -u origin {branch-name} +(cd "{docs_repo_path}" && python3 "$PUBLISH_SCRIPT" push --remote origin --branch {branch-name}) ``` -Prepare the PR description and save it to `.artifacts/prd/{issue-key}/04-pr-description.md` -(in the source repo's artifact directory): +Prepare the PR description (AI-dependent — summarize the PRD content) and +save it to `.artifacts/prd/{issue-key}/04-pr-description.md` (in the source +repo's artifact directory): ```markdown ## PRD: {title} @@ -204,26 +247,55 @@ Prepare the PR description and save it to `.artifacts/prd/{issue-key}/04-pr-desc Determine `{owner}/{repo}` from the `docs_repo_remote` in `.artifacts/config.json` (e.g., `git@github.com:org/planning-docs.git` → `org/planning-docs`), then -create the draft PR. If `{issue-key}` is a Jira key, prefix the title -with it (`{issue-key}: PRD - {title}`); otherwise use `PRD: {title}`. +check for an existing PR before creating one. If `{issue-key}` is a Jira +key, prefix the title with it (`{issue-key}: PRD - {title}`); otherwise +use `PRD: {title}`. + +First, check whether a PR already exists for this branch: ```bash -gh pr create --draft --repo {owner}/{repo} --base {base-branch} --head {branch-name} --title "{issue-key}: PRD - {title}" --body-file .artifacts/prd/{issue-key}/04-pr-description.md +python3 "$PUBLISH_SCRIPT" check-existing --repo {owner}/{repo} --head {branch-name} ``` +If exit code is 5, a PR already exists — skip to Step 6 and report its +URL. Parse the PR number from the returned JSON. If the command fails +(non-zero exit other than 5), stop and report the error. If exit code +is 0, create a new PR: + +```bash +# When {issue-key} is a Jira key (e.g., EDM-1471): +python3 "$PUBLISH_SCRIPT" create-pr \ + --repo {owner}/{repo} \ + --base {base-branch} \ + --head {branch-name} \ + --title "{issue-key}: PRD - {title}" \ + --body-file .artifacts/prd/{issue-key}/04-pr-description.md \ + --draft + +# When no issue key exists: +python3 "$PUBLISH_SCRIPT" create-pr \ + --repo {owner}/{repo} \ + --base {base-branch} \ + --head {branch-name} \ + --title "PRD: {title}" \ + --body-file .artifacts/prd/{issue-key}/04-pr-description.md \ + --draft +``` + +The script prints the PR URL on stdout. Parse the PR number from the URL path. + ### Step 6: Save Publish Metadata -Write `.artifacts/prd/{issue-key}/publish-metadata.json` to record the -file path and PR details for use by `/revise` and `/respond`: - -```json -{ - "release": "{release}", - "feature": "{feature}", - "prd_file_path": "{release}/{feature}/prd.md", - "pr_number": {pr-number}, - "branch": "{branch-name}" -} +Save metadata for use by `/revise` and `/respond`: + +```bash +python3 "$PUBLISH_SCRIPT" save-metadata \ + --file .artifacts/prd/{issue-key}/publish-metadata.json \ + release={release} \ + feature={feature} \ + prd_file_path={release}/{feature}/prd.md \ + pr_number={pr-number} \ + branch={branch-name} ``` ### Step 7: Report to User From 636e4bca5b42337731fe023348c5926245f2b485 Mon Sep 17 00:00:00 2001 From: Chai Bot <chai-bot@redhat.com> Date: Wed, 9 Sep 2026 21:25:38 +0000 Subject: [PATCH 2/2] Fix review feedback from PR #119 - Fix duplicate branch prefix bug: BRANCH_NAME already includes the prefix (bugfix/, docs/), so subsequent commands must not add it again (bugfix/skills/pr.md, docs-writer/skills/create-mr.md) - Fix case-sensitive variable: ${ticket_id} -> ${TICKET_ID} in docs-writer/skills/create-mr.md - Add has_untracked to dirty-state check in implement/skills/publish.md (was already fixed in bugfix and e2e but missed here) - Document exit code 2 (argparse) in publish.py header - Guard empty MR list in _check_existing_gitlab to skip project ID resolution when no MRs match the source branch - Always pass --description to glab mr create to prevent interactive prompting - Enrich docstrings on all public functions in publish.py - Add tests for empty-MR-list guard and always-pass-description behavior Assisted-by: Claude <noreply@anthropic.com> --- _shared/scripts/publish.py | 56 +++++++++++++++++++++++++++------ _shared/scripts/test_publish.py | 32 +++++++++++++++++++ bugfix/skills/pr.md | 18 +++++------ docs-writer/skills/create-mr.md | 28 ++++++++--------- implement/skills/publish.md | 8 +++-- 5 files changed, 107 insertions(+), 35 deletions(-) diff --git a/_shared/scripts/publish.py b/_shared/scripts/publish.py index 75dec1ae..b85e36ee 100644 --- a/_shared/scripts/publish.py +++ b/_shared/scripts/publish.py @@ -31,6 +31,7 @@ Exit codes: 0 -- success 1 -- missing argument or configuration error + 2 -- invalid arguments (from argparse) 3 -- push failed 4 -- PR/MR creation failed 5 -- existing PR/MR found (check-existing only; prints JSON on stdout) @@ -79,7 +80,16 @@ def run( capture: bool = True, check: bool = False, ) -> subprocess.CompletedProcess[str]: - """Run a subprocess with text output.""" + """Run a subprocess with text mode and optional output capture. + + Args: + cmd: Command and arguments to execute. + capture: If True, capture stdout and stderr (default: True). + check: If True, raise CalledProcessError on non-zero exit. + + Returns: + The completed process with text-mode stdout/stderr. + """ return subprocess.run( cmd, capture_output=capture, @@ -227,7 +237,11 @@ def cmd_check_existing(args: argparse.Namespace) -> int: def _check_existing_github(repo: str, head: str) -> int: - """GitHub check-existing: use gh pr list with headRepositoryOwner filtering.""" + """Check for an existing GitHub PR using gh pr list. + + Supports fork-aware matching via headRepositoryOwner filtering + when ``head`` is in ``owner:branch`` format. + """ if ":" in head: # owner:branch format -- gh pr list --head does not support this # syntax. Search by branch name and filter by head repo owner. @@ -286,7 +300,12 @@ def _check_existing_github(repo: str, head: str) -> int: def _check_existing_gitlab(repo: str, head: str) -> int: - """GitLab check-existing: use glab mr list with source_project_id filtering.""" + """Check for an existing GitLab MR using glab mr list. + + Supports fork-aware matching via source_project_id filtering + when ``head`` is in ``project:branch`` format. Skips the project + ID resolution when no MRs match the source branch. + """ source_branch = head source_project = "" @@ -311,6 +330,11 @@ def _check_existing_gitlab(repo: str, head: str) -> int: return EXIT_ARG_ERROR # unreachable if source_project: + # Skip fork project ID resolution when no MRs matched the branch + if not mrs: + info(f"No existing PR/MR found for head={head} on {repo}") + return EXIT_SUCCESS + # Resolve the fork's numeric project ID to filter by source_project_id encoded = source_project.replace("/", "%2F") r = run(["glab", "api", f"projects/{encoded}", "--jq", ".id"]) @@ -346,7 +370,11 @@ def _check_existing_gitlab(repo: str, head: str) -> int: # --------------------------------------------------------------------------- def cmd_create_pr(args: argparse.Namespace) -> int: - """Create a GitHub pull request via the gh CLI.""" + """Create a GitHub pull request via the gh CLI. + + On success, prints the new PR URL on stdout. + Exits with EXIT_CREATE_FAIL (4) if ``gh pr create`` fails. + """ if not args.base: fail("Missing required argument: --base") if not args.head: @@ -395,7 +423,12 @@ def cmd_create_pr(args: argparse.Namespace) -> int: # --------------------------------------------------------------------------- def cmd_create_mr(args: argparse.Namespace) -> int: - """Create a GitLab merge request via the glab CLI.""" + """Create a GitLab merge request via the glab CLI. + + Always passes ``--description`` to prevent interactive prompting. + On success, prints the new MR URL on stdout. + Exits with EXIT_CREATE_FAIL (4) if ``glab mr create`` fails. + """ if not args.source: fail("Missing required argument: --source") if not args.target: @@ -427,8 +460,8 @@ def cmd_create_mr(args: argparse.Namespace) -> int: fail(f"create-mr: description file not found: {args.desc_file}") description = desc_path.read_text(encoding="utf-8") - if description: - cmd.extend(["--description", description]) + # Always pass --description to prevent glab from prompting interactively + cmd.extend(["--description", description]) info(f"Creating MR: {args.title}") result = run(cmd, capture=True) @@ -449,7 +482,12 @@ def cmd_create_mr(args: argparse.Namespace) -> int: # --------------------------------------------------------------------------- def cmd_save_metadata(args: argparse.Namespace) -> int: - """Write a JSON metadata file from key=value pairs.""" + """Write a sorted JSON metadata file from positional key=value pairs. + + All values are stored as JSON strings to avoid leading-zero truncation + and to keep the output type-stable. Keys are sorted alphabetically + for deterministic output. + """ file_path = args.file pairs: list[str] = args.pair or [] @@ -601,7 +639,7 @@ def build_parser() -> argparse.ArgumentParser: def main(argv: list[str] | None = None) -> int: - """Entry point.""" + """Parse arguments and dispatch to the appropriate subcommand handler.""" parser = build_parser() args = parser.parse_args(argv) diff --git a/_shared/scripts/test_publish.py b/_shared/scripts/test_publish.py index 7aaf0719..5183ea22 100644 --- a/_shared/scripts/test_publish.py +++ b/_shared/scripts/test_publish.py @@ -542,6 +542,20 @@ def test_gitlab_fork_filter_by_project_id(self, mock_run: mock.Mock) -> None: result = json.loads(buf.getvalue()) self.assertEqual(result["iid"], 2) + @mock.patch.object(publish, "run") + def test_gitlab_fork_empty_mrs_skips_project_id(self, mock_run: mock.Mock) -> None: + """Fork-aware GitLab: empty MR list skips project ID resolution.""" + mock_run.return_value = subprocess.CompletedProcess( + [], 0, "[]", "", + ) + code = publish.main([ + "check-existing", "--repo", "grp/proj", + "--head", "jsmith/proj:feat/x", "--platform", "gitlab", + ]) + self.assertEqual(code, publish.EXIT_SUCCESS) + # Should only call glab mr list -- NOT glab api projects/... + self.assertEqual(mock_run.call_count, 1) + # --------------------------------------------------------------------------- # JSON encoding edge case tests @@ -734,6 +748,24 @@ def test_desc_file_read(self, mock_run: mock.Mock) -> None: finally: os.unlink(f.name) + @mock.patch.object(publish, "run") + def test_always_passes_description(self, mock_run: mock.Mock) -> None: + """glab mr create must always receive --description to prevent prompts.""" + mock_run.return_value = subprocess.CompletedProcess( + [], 0, "https://gitlab.com/grp/proj/-/merge_requests/1\n", "", + ) + import io + buf = io.StringIO() + with mock.patch("sys.stdout", buf): + publish.main([ + "create-mr", "--source", "docs/fix", + "--target", "main", "--title", "T", + ]) + call_args = mock_run.call_args[0][0] + self.assertIn("--description", call_args) + idx = call_args.index("--description") + self.assertEqual(call_args[idx + 1], "") + # --------------------------------------------------------------------------- # Push tests (with subprocess mocking) diff --git a/bugfix/skills/pr.md b/bugfix/skills/pr.md index 013de8f3..9466d0e7 100644 --- a/bugfix/skills/pr.md +++ b/bugfix/skills/pr.md @@ -359,7 +359,7 @@ git rebase fork/main ### Step 4: Create a Branch ```bash -git checkout -b bugfix/BRANCH_NAME +git checkout -b BRANCH_NAME ``` Branch naming conventions: @@ -453,7 +453,7 @@ to write an accurate commit message. Don't make up details. ### Step 8: Push to Fork ```bash -python3 "$PUBLISH_SCRIPT" push --remote fork --branch bugfix/BRANCH_NAME +python3 "$PUBLISH_SCRIPT" push --remote fork --branch BRANCH_NAME ``` **If the script exits with code 3 (push failed):** @@ -470,14 +470,14 @@ access. Please run: `git push -u fork BRANCH_NAME`" ### Step 9: Create the Draft PR **Check for an existing PR** before attempting creation. Use -`FORK_OWNER:bugfix/BRANCH_NAME` so the check matches only PRs from -this fork (plain `bugfix/BRANCH_NAME` would match any fork's branch +`FORK_OWNER:BRANCH_NAME` so the check matches only PRs from +this fork (plain `BRANCH_NAME` would match any fork's branch with the same name): ```bash python3 "$PUBLISH_SCRIPT" check-existing \ --repo UPSTREAM_OWNER/REPO \ - --head FORK_OWNER:bugfix/BRANCH_NAME + --head FORK_OWNER:BRANCH_NAME ``` If exit code is 5, a PR already exists — skip to Step 10 and report its @@ -493,7 +493,7 @@ If the `--body-file` artifact exists: ```bash python3 "$PUBLISH_SCRIPT" create-pr \ --repo UPSTREAM_OWNER/REPO \ - --head FORK_OWNER:bugfix/BRANCH_NAME \ + --head FORK_OWNER:BRANCH_NAME \ --base main \ --title "[ISSUE_KEY]: short description in lowercase" \ --body-file .artifacts/bugfix/{issue}/pr-description.md \ @@ -506,7 +506,7 @@ see the template in this skill's Notes section) and pass it with `--body`: ```bash python3 "$PUBLISH_SCRIPT" create-pr \ --repo UPSTREAM_OWNER/REPO \ - --head FORK_OWNER:bugfix/BRANCH_NAME \ + --head FORK_OWNER:BRANCH_NAME \ --base main \ --title "[ISSUE_KEY]: short description in lowercase" \ --body "PR_BODY_TEXT" \ @@ -533,7 +533,7 @@ do NOT debug further, do NOT fall back to a patch file. Instead: 2. **Give the user a pre-filled GitHub compare URL:** ```text - https://github.com/UPSTREAM_OWNER/REPO/compare/main...FORK_OWNER:bugfix/BRANCH_NAME?expand=1 + https://github.com/UPSTREAM_OWNER/REPO/compare/main...FORK_OWNER:BRANCH_NAME?expand=1 ``` This URL opens GitHub's "Open a pull request" form with the branches @@ -547,7 +547,7 @@ do NOT debug further, do NOT fall back to a patch file. Instead: it as a draft. **If "branch not found"**: The push in Step 8 may have failed silently. -Verify with `git ls-remote fork bugfix/BRANCH_NAME`. +Verify with `git ls-remote fork BRANCH_NAME`. ### Step 10: Confirm and Report diff --git a/docs-writer/skills/create-mr.md b/docs-writer/skills/create-mr.md index cf49a87a..964900fc 100644 --- a/docs-writer/skills/create-mr.md +++ b/docs-writer/skills/create-mr.md @@ -227,7 +227,7 @@ git remote add fork https://gitlab.cee.redhat.com/FORK_PROJECT.git ### Step 4: Create a Branch ```bash -git checkout -b docs/BRANCH_NAME +git checkout -b BRANCH_NAME ``` Branch naming conventions: @@ -270,7 +270,7 @@ discovered from `git remote -v` — typically `origin` for direct push or # Set PUSH_REMOTE based on the push strategy determined in Step 2/3: # - Direct push: PUSH_REMOTE is the remote pointing to UPSTREAM_PROJECT # - Fork workflow: PUSH_REMOTE is the remote pointing to FORK_PROJECT -python3 "$PUBLISH_SCRIPT" push --remote "$PUSH_REMOTE" --branch "docs/$BRANCH_NAME" +python3 "$PUBLISH_SCRIPT" push --remote "$PUSH_REMOTE" --branch "$BRANCH_NAME" ``` **If the script exits with code 3 (push failed):** @@ -284,7 +284,7 @@ python3 "$PUBLISH_SCRIPT" push --remote "$PUSH_REMOTE" --branch "docs/$BRANCH_NA **MR title format:** Use `[TICKET_ID]: short description in lowercase`. **Building the description:** Use the MR description prepared by the `/apply` -phase at `.artifacts/${ticket_id}/04-mr-description.md`. If the file does not +phase at `.artifacts/${TICKET_ID}/04-mr-description.md`. If the file does not exist, build the description (AI-dependent) from the context artifact (`01-context.md`) and plan artifact (`02-plan.md`). @@ -292,10 +292,10 @@ exist, build the description (AI-dependent) from the context artifact ```bash # Direct push: -python3 "$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "docs/$BRANCH_NAME" --platform gitlab +python3 "$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "$BRANCH_NAME" --platform gitlab # Fork workflow (project:branch filters by source project to avoid cross-fork false matches): -python3 "$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "FORK_PROJECT:docs/$BRANCH_NAME" --platform gitlab +python3 "$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "FORK_PROJECT:$BRANCH_NAME" --platform gitlab ``` If exit code is 5, an MR already exists — skip to Step 8 and report its @@ -306,10 +306,10 @@ the error. If exit code is 0, create a new MR: ```bash python3 "$PUBLISH_SCRIPT" create-mr \ - --source "docs/$BRANCH_NAME" \ + --source "$BRANCH_NAME" \ --target main \ --title "[TICKET_ID]: short description" \ - --desc-file ".artifacts/${ticket_id}/04-mr-description.md" \ + --desc-file ".artifacts/${TICKET_ID}/04-mr-description.md" \ --draft ``` @@ -321,27 +321,27 @@ If no description file exists, use `--description` with inline text instead. python3 "$PUBLISH_SCRIPT" create-mr \ --project UPSTREAM_PROJECT \ --head FORK_PROJECT \ - --source "docs/$BRANCH_NAME" \ + --source "$BRANCH_NAME" \ --target main \ --title "[TICKET_ID]: short description" \ - --desc-file ".artifacts/${ticket_id}/04-mr-description.md" \ + --desc-file ".artifacts/${TICKET_ID}/04-mr-description.md" \ --draft ``` **If the script exits with code 4 (MR creation failed):** -1. **Write the MR description** to `.artifacts/${ticket_id}/04-mr-description.md` +1. **Write the MR description** to `.artifacts/${TICKET_ID}/04-mr-description.md` 2. **Give the user a pre-filled GitLab MR URL:** Direct push: ```text - https://gitlab.cee.redhat.com/UPSTREAM_PROJECT/-/merge_requests/new?merge_request[source_branch]=docs/BRANCH_NAME&merge_request[target_branch]=main + https://gitlab.cee.redhat.com/UPSTREAM_PROJECT/-/merge_requests/new?merge_request[source_branch]=BRANCH_NAME&merge_request[target_branch]=main ``` Fork: ```text - https://gitlab.cee.redhat.com/UPSTREAM_PROJECT/-/merge_requests/new?merge_request[source_project_id]=FORK_PROJECT_ID&merge_request[source_branch]=docs/BRANCH_NAME&merge_request[target_branch]=main + https://gitlab.cee.redhat.com/UPSTREAM_PROJECT/-/merge_requests/new?merge_request[source_project_id]=FORK_PROJECT_ID&merge_request[source_branch]=BRANCH_NAME&merge_request[target_branch]=main ``` 3. **Provide the MR title and description** for the user to paste in. @@ -369,7 +369,7 @@ Diagnose it using the Error Recovery table and retry. If `glab mr create` fails but the branch is pushed: -1. **Write the MR description** to `.artifacts/${ticket_id}/04-mr-description.md` +1. **Write the MR description** to `.artifacts/${TICKET_ID}/04-mr-description.md` 2. **Provide the new MR URL** for the user to open in their browser 3. **Show the MR title and description** for the user to paste in @@ -385,7 +385,7 @@ If push fails due to network or auth restrictions: Only if ALL of the above fail: 1. Generate a patch: `git diff > docs-changes.patch` -2. Write it to `.artifacts/${ticket_id}/docs-changes.patch` +2. Write it to `.artifacts/${TICKET_ID}/docs-changes.patch` 3. Explain how to apply it: `git apply docs-changes.patch` 4. **Acknowledge this is a degraded experience** diff --git a/implement/skills/publish.md b/implement/skills/publish.md index 27c0902e..9427c16b 100644 --- a/implement/skills/publish.md +++ b/implement/skills/publish.md @@ -78,9 +78,11 @@ Verify readiness: python3 "$PUBLISH_SCRIPT" preflight --platform github ``` - Parse the output to confirm `auth_ok=true` and check for - `has_uncommitted=true` or `has_staged=true`. If there are uncommitted - changes, ask the user how to proceed. + Parse the output to confirm `auth_ok=true`. If `auth_ok=false`, stop + and tell the user to authenticate first. Check for + `has_uncommitted=true`, `has_staged=true`, or `has_untracked=true`. + If there are uncommitted or untracked changes, ask the user how to + proceed. ### Step 2: Cross-Cutting Review