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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 71 additions & 37 deletions src/tenzir_ship/cli/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from __future__ import annotations

import re
import sys
from dataclasses import dataclass
from importlib.metadata import PackageNotFoundError, version as metadata_version
Expand All @@ -11,6 +12,7 @@
Callable,
Iterable,
Mapping,
NamedTuple,
Optional,
TypeVar,
)
Expand Down Expand Up @@ -64,6 +66,8 @@
"_command_help_text",
"_format_author",
"_format_section_title",
"PrRef",
"_parse_pr_refs",
"_parse_pr_numbers",
"_build_prs_structured",
"_build_authors_structured",
Expand Down Expand Up @@ -567,45 +571,79 @@ def _type_emoji(entry_type: str, *, include_emoji: bool = True) -> str:
return "\u2022"


def _parse_pr_numbers(metadata: Mapping[str, Any]) -> list[int]:
"""Extract PR numbers from metadata, handling various formats."""
GITHUB_PR_URL_RE = re.compile(
r"^https://github\.com/(?P<repo>[^/\s]+/[^/\s]+)/pull/(?P<number>\d+)/?$"
)


class PrRef(NamedTuple):
"""A pull-request reference.

A bare number is relative to the project's configured repository, so its
URL can only be built when that is known. An explicit URL carries its own
repository, which is what lets entries reference pull requests outside the
configured one.
"""

number: int | None
url: str | None

@property
def label(self) -> str:
return f"#{self.number}" if self.number is not None else (self.url or "")


def _parse_pr_ref(item: Any, repository: str | None) -> PrRef | None:
"""Parse a single PR reference, or return None if it is not one."""
if isinstance(item, bool):
return None
if isinstance(item, int):
number = item
else:
if not isinstance(item, str):
return None
text = item.strip()
if not text:
return None
match = GITHUB_PR_URL_RE.match(text)
if match:
# Keep the URL verbatim: it may point at a different repository
# than the configured one, which is the whole point of allowing it.
return PrRef(number=int(match.group("number")), url=text)
Comment on lines +608 to +612

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Accept URL refs through the entry-creation API

When a foreign-repository PR is supplied through the supported creation surfaces (tenzir-ship add --pr <URL> or Changelog.add(prs=[<URL>])), src/tenzir_ship/cli/_add.py:276-283 still passes every value to int() and raises β€œmust be numeric,” so this parser only benefits hand-edited entry files. The bundled entry workflow likewise documents only the add --pr path in skills/tenzir-ship/references/add-changelog-entry.md:160-177; route entry creation through the URL-aware parsing/storage path and update that reference so users can actually create the newly supported metadata.

AGENTS.md reference: AGENTS.md:L52-L57

Useful? React with πŸ‘Β / πŸ‘Ž.

Comment on lines +608 to +612

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Allow URL references in the entry schema

When an entry uses the newly accepted full GitHub PR URL, validate_entry() still validates it against schemas/changelog-entry.schema.json, whose prs string pattern accepts only numeric references such as #42. Consequently, tenzir-ship validate reports every URL-backed entry as invalid, so changelogs cannot adopt this metadata while retaining validation or release quality gates; extend the entry schema and its validation tests to accept the same URL syntax as this parser.

Useful? React with πŸ‘Β / πŸ‘Ž.

text = text.lstrip("#")
if not text.isdigit():
return None
number = int(text)
url = f"https://github.com/{repository}/pull/{number}" if repository else None
return PrRef(number=number, url=url)


def _parse_pr_refs(metadata: Mapping[str, Any], repository: str | None = None) -> list[PrRef]:
"""Extract PR references from metadata, handling numbers and full URLs."""
raw = metadata.get("prs")
if raw is None:
return []
if isinstance(raw, int):
return [raw]
if isinstance(raw, str):
raw = raw.strip()
if not raw:
return []
if raw.startswith("#"):
raw = raw[1:]
try:
return [int(raw)]
except ValueError:
return []
if isinstance(raw, list):
result: list[int] = []
for item in raw:
if isinstance(item, int):
result.append(item)
elif isinstance(item, str):
item = item.strip().lstrip("#")
if item.isdigit():
result.append(int(item))
return result
return []
items = raw if isinstance(raw, list) else [raw]
refs = [_parse_pr_ref(item, repository) for item in items]
return [ref for ref in refs if ref is not None]


def _parse_pr_numbers(metadata: Mapping[str, Any]) -> list[int]:
"""Extract PR numbers from metadata, handling various formats."""
return [ref.number for ref in _parse_pr_refs(metadata) if ref.number is not None]


def _build_prs_structured(
metadata: Mapping[str, Any], config: Config
) -> list[dict[str, str | int]]:
"""Build structured PR metadata for JSON export."""
prs: list[dict[str, str | int]] = []
for num in _parse_pr_numbers(metadata):
entry: dict[str, str | int] = {"number": num}
if config.repository:
entry["url"] = f"https://github.com/{config.repository}/pull/{num}"
for ref in _parse_pr_refs(metadata, config.repository):
entry: dict[str, str | int] = {}
if ref.number is not None:
entry["number"] = ref.number
if ref.url:
entry["url"] = ref.url
prs.append(entry)
return prs

Expand Down Expand Up @@ -699,16 +737,12 @@ def _collect_author_pr_text(
author_handles = [_format_author(author, explicit_links=explicit_links) for author in authors]
author_text = _join_with_conjunction(author_handles)

prs = _parse_pr_numbers(metadata)

repo = config.repository
pr_refs: list[str] = []
for pr in prs:
label = f"#{pr}"
if explicit_links and repo:
pr_refs.append(f"[{label}](https://github.com/{repo}/pull/{pr})")
for ref in _parse_pr_refs(metadata, config.repository):
if explicit_links and ref.url:
pr_refs.append(f"[{ref.label}]({ref.url})")
else:
pr_refs.append(label)
pr_refs.append(ref.label)
pr_text = _join_with_conjunction(pr_refs)

return author_text, pr_text
Expand Down
80 changes: 80 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from tenzir_ship import __version__
from tenzir_ship.cli import INFO_PREFIX, cli, main
from tenzir_ship.cli._core import _parse_pr_numbers, _parse_pr_refs
from tenzir_ship.cli._core import create_cli_context
from tenzir_ship.cli._show import _collect_unused_entries_for_release
from tenzir_ship.config import Config, ReleaseConfig, load_config, save_config
Expand Down Expand Up @@ -9145,3 +9146,82 @@ def test_validate_rejects_duplicate_ids_within_one_manifest(tmp_path: Path) -> N

assert result.exit_code != 0
assert "non-unique elements" in result.output


def test_parse_pr_refs_accepts_numbers_and_full_urls() -> None:
"""PR references may be bare numbers or full URLs to any repository."""
refs = _parse_pr_refs(
{
"prs": [
6368,
"#42",
"https://github.com/tenzir/tenzir/pull/6441",
"https://github.com/tenzir/mono/pull/35/",
"not-a-pr",
]
},
"tenzir/mono",
)

assert [ref.number for ref in refs] == [6368, 42, 6441, 35]
# Bare numbers resolve against the configured repository; explicit URLs
# keep whatever repository they name.
assert refs[0].url == "https://github.com/tenzir/mono/pull/6368"
assert refs[1].url == "https://github.com/tenzir/mono/pull/42"
assert refs[2].url == "https://github.com/tenzir/tenzir/pull/6441"
assert refs[3].url == "https://github.com/tenzir/mono/pull/35/"


def test_parse_pr_refs_without_repository_keeps_explicit_urls() -> None:
"""A project without a configured repository can still carry URLs."""
refs = _parse_pr_refs(
{"prs": [7, "https://github.com/tenzir/tenzir/pull/6441"]},
None,
)

assert refs[0].url is None
assert refs[1].url == "https://github.com/tenzir/tenzir/pull/6441"


def test_parse_pr_numbers_still_returns_plain_numbers() -> None:
"""The display-only helper keeps its list[int] contract."""
assert _parse_pr_numbers({"prs": [1, "#2", "https://github.com/tenzir/tenzir/pull/3"]}) == [
1,
2,
3,
]


def test_explicit_links_uses_the_entry_url_for_foreign_repositories(
tmp_path: Path,
) -> None:
"""A full URL must survive rendering instead of being rebuilt from config."""
runner = CliRunner()
project_dir = tmp_path / "project"
project_dir.mkdir()
(project_dir / "config.yaml").write_text(
"id: test-project\nname: Test Project\nrepository: tenzir/mono\n",
encoding="utf-8",
)
entry_dir = project_dir / "unreleased"
entry_dir.mkdir(parents=True)
(entry_dir / "foreign-pr.md").write_text(
"---\n"
"title: Foreign PR reference\n"
"type: feature\n"
"authors:\n"
" - someone\n"
"prs:\n"
" - https://github.com/tenzir/tenzir/pull/6441\n"
"---\n\nBody.\n",
encoding="utf-8",
)

result = runner.invoke(
cli,
["--root", str(project_dir), "show", "-m", "--explicit-links", "unreleased"],
)

assert result.exit_code == 0, result.output
assert "[#6441](https://github.com/tenzir/tenzir/pull/6441)" in result.output
assert "tenzir/mono/pull/6441" not in result.output
Loading