Skip to content
Merged
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
13 changes: 13 additions & 0 deletions .github/workflows/repo-policy-sync.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,19 @@ jobs:
uses: actions/checkout@v7.0.0
- name: Setup uv
uses: astral-sh/setup-uv@v7
- name: Configure bot git identity
run: |
git config --global user.name "eclipse-score-bot"
git config --global user.email "187756813+eclipse-score-bot@users.noreply.github.com"
- name: Require bot token for apply mode
if: env.MODE == 'apply'
env:
SCORE_BOT_CLASSIC_PAT: ${{ secrets.SCORE_BOT_CLASSIC_PAT }}
run: |
if [[ -z "$SCORE_BOT_CLASSIC_PAT" ]]; then
echo "SCORE_BOT_CLASSIC_PAT is required to apply policies across repositories." >&2
exit 1
fi
- name: Run policy sync
id: sync
run: >-
Expand Down
63 changes: 51 additions & 12 deletions repo_policy_sync/src/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,15 +540,45 @@ def create_pull_request(
# and could be duplicated or overwritten. Only the cosmetic "automation"
# label may fail without aborting.
warnings: list[str] = []
pull_request = PullRequest(number=_pull_request_number(output), url=output)
for label in AUTOMATION_LABELS:
if label == TOOL_SLUG:
self._run(["gh", "pr", "edit", output, "--add-label", label])
self._add_pull_request_label(
repository=repository, pull_request=pull_request, label=label
)
continue
try:
self._run(["gh", "pr", "edit", output, "--add-label", label])
self._add_pull_request_label(
repository=repository, pull_request=pull_request, label=label
)
except CommandError as exc:
warnings.append(f"label {label!r} was not applied: {exc}")
return PullRequest(number=0, url=output, warnings=tuple(warnings))
return PullRequest(
number=pull_request.number, url=output, warnings=tuple(warnings)
)

def _add_pull_request_label(
self, *, repository: str, pull_request: PullRequest, label: str
) -> None:
"""Add a label through REST with the cross-repository bot's repo scope.

GitHub CLI's pull-request commands are GraphQL-backed and require
``read:org`` for organization-owned repositories. The bot deliberately
has only the narrower ``repo`` scope, which is sufficient for this
REST endpoint.
"""

self._run(
[
"gh",
"api",
"--method",
"POST",
f"/repos/{repository}/issues/{pull_request.number}/labels",
"-f",
f"labels[]={label}",
]
)

def _ensure_automation_labels(self, *, repository: str) -> None:
"""Create the labels applied to generated pull requests when they are absent."""
Expand Down Expand Up @@ -615,18 +645,20 @@ def update_pull_request(
) -> None:
"""Keep an existing policy-owned pull request's explanation current."""

# GitHub CLI's pull-request edit command is GraphQL-backed and needs
# read:org for organization-owned repositories. The cross-repository
# bot deliberately has only repo scope, which is sufficient here.
self._run(
[
"gh",
"pr",
"edit",
pull_request.url,
"--repo",
repository,
"--title",
policy.title,
"--body",
_pull_request_body(policy, changes, head_oid=head_oid, failure=failure),
"api",
"--method",
"PATCH",
f"/repos/{repository}/pulls/{pull_request.number}",
"-f",
f"title={policy.title}",
"-f",
f"body={_pull_request_body(policy, changes, head_oid=head_oid, failure=failure)}",
]
)

Expand Down Expand Up @@ -746,6 +778,13 @@ def _policy_head_marker_from_body(body: str) -> str | None:
return match.group(1) if match else None


def _pull_request_number(url: str) -> int:
match = re.search(r"/pull/(\d+)/?$", url)
if match is None:
raise CommandError(f"gh did not return a valid pull-request URL: {url}")
return int(match.group(1))


def _pull_request_body(
policy: Policy,
changes: tuple[Change, ...],
Expand Down
19 changes: 4 additions & 15 deletions repo_policy_sync/src/reporting.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,9 @@ def _markdown_pr_badge(status: str, url: str) -> str:


def _markdown_details_section(outcomes: tuple[RepositoryOutcome, ...]) -> list[str]:
detailed = tuple(
outcome for outcome in outcomes if _markdown_details(outcome) != "—"
)
"""Render error details without duplicating the policy pull-request list."""

detailed = tuple(outcome for outcome in outcomes if outcome.error)
if not detailed:
Comment on lines 247 to 251
return []

Expand All @@ -261,23 +261,12 @@ def _markdown_details_section(outcomes: tuple[RepositoryOutcome, ...]) -> list[s
lines.append(
f"- `{_markdown_cell(outcome.repository)}` / "
f"`{_markdown_cell(outcome.policy_id)}`: "
f"{_markdown_cell(_markdown_details(outcome))}"
f"{_markdown_cell(outcome.error or 'unknown error')}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So anything else except policy_id and repository will now not be in the markdown cell anymore.
That is wanted?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes, intentional. too much noise. link to PR is there, no need to write here what was changed.

)
lines.extend(["", "</details>"])
return lines


def _markdown_details(outcome: RepositoryOutcome) -> str:
if outcome.error:
return outcome.error
if outcome.status == "skipped":
return "No default branch; evaluation skipped."
details = _format_changes(outcome.changes)
if outcome.warnings:
details = "; ".join(part for part in (details, *outcome.warnings) if part)
return details or "—"


def _markdown_url(value: str) -> str:
return value.replace("(", "%28").replace(")", "%29").replace("\n", "")

Expand Down
77 changes: 45 additions & 32 deletions repo_policy_sync/tests/test_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,28 +468,16 @@ def run(command: list[str]) -> str:
] == [
"/repos/owner/repo/labels",
"/repos/owner/repo/labels",
"/repos/owner/repo/issues/1/labels",
"/repos/owner/repo/issues/1/labels",
]
assert [
command[6]
command[-1]
for command in commands
if command[:4] == ["gh", "api", "--method", "POST"]
] == [
"name=automation",
"name=repo-policy-sync",
]
assert [
command[8]
for command in commands
if command[:4] == ["gh", "api", "--method", "POST"]
if command[4] == "/repos/owner/repo/issues/1/labels"
] == [
"color=EDEDED",
"color=EDEDED",
]
assert [
command[-1] for command in commands if command[:3] == ["gh", "pr", "edit"]
] == [
"automation",
"repo-policy-sync",
"labels[]=automation",
"labels[]=repo-policy-sync",
]


Expand All @@ -508,7 +496,7 @@ def run(command: list[str]) -> str:
return '[[{"name":"automation"},{"name":"repo-policy-sync"}]]'
if command[:3] == ["gh", "pr", "create"]:
return "https://github.example/owner/repo/pull/1\n"
if command[:3] == ["gh", "pr", "edit"]:
if command[:2] == ["gh", "api"]:
return ""
raise AssertionError(command)

Expand All @@ -524,9 +512,12 @@ def run(command: list[str]) -> str:
head_oid="a" * 40,
)

assert not any(
command[:4] == ["gh", "api", "--method", "POST"] for command in commands
)
assert not any(command[4] == "/repos/owner/repo/labels" for command in commands)
assert [
command[-1]
for command in commands
if command[4] == "/repos/owner/repo/issues/1/labels"
] == ["labels[]=automation", "labels[]=repo-policy-sync"]


def test_create_pull_request_fails_when_tool_label_cannot_be_applied(
Expand All @@ -543,9 +534,31 @@ def run(command: list[str]) -> str:
return '[[{"name":"automation"},{"name":"repo-policy-sync"}]]'
if command[:3] == ["gh", "pr", "create"]:
return "https://github.example/owner/repo/pull/1\n"
if command[:3] == ["gh", "pr", "edit"] and command[-1] == "repo-policy-sync":
raise CommandError("gh pr edit: permission denied")
if command[:3] == ["gh", "pr", "edit"]:
if (
command[:5]
== [
"gh",
"api",
"--method",
"POST",
"/repos/owner/repo/issues/1/labels",
]
and command[-1] == "labels[]=automation"
):
return ""
if (
command[:5]
== [
"gh",
"api",
"--method",
"POST",
"/repos/owner/repo/issues/1/labels",
]
and command[-1] == "labels[]=repo-policy-sync"
):
raise CommandError("gh api: permission denied")
if command[:2] == ["gh", "api"]:
return ""
raise AssertionError(command)

Expand Down Expand Up @@ -578,7 +591,7 @@ def run(command: list[str]) -> str:
return '[[{"name":"automation"},{"name":"repo-policy-sync"}]]'
if command[:3] == ["gh", "pr", "create"]:
return "https://github.example/owner/repo/pull/1\n"
if command[:3] == ["gh", "pr", "edit"]:
if command[:2] == ["gh", "api"]:
return ""
raise AssertionError(command)

Expand Down Expand Up @@ -686,12 +699,12 @@ def record(command: list[str]) -> str:

assert commands[0][:7] == [
"gh",
"pr",
"edit",
"https://github.example/owner/repo/pull/1",
"--repo",
"owner/repo",
"--title",
"api",
"--method",
"PATCH",
"/repos/owner/repo/pulls/1",
"-f",
"title=Current title",
]
assert "## Policy" in commands[0][-1]

Expand Down
6 changes: 3 additions & 3 deletions repo_policy_sync/tests/test_reporting.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ def test_render_markdown_distinguishes_compliance_and_policy_pr_status() -> None
assert "`✅ 1 closed`" in output


def test_render_markdown_uses_policies_as_matrix_columns_and_keeps_details_compact() -> (
def test_render_markdown_uses_policies_as_matrix_columns_and_keeps_only_errors_in_details() -> (
None
):
report = RunReport(
Expand Down Expand Up @@ -360,7 +360,7 @@ def test_render_markdown_uses_policies_as_matrix_columns_and_keeps_details_compa
assert "| Repository | policy-a | policy-b |" in output
assert "| first | ❌ | N/A |" in output
assert "| second | ✅ | ⚠️ |" in output
assert "<summary>Details (2)</summary>" in output
assert ".gitignore: add generated files" in output
assert "<summary>Details (1)</summary>" in output
assert ".gitignore: add generated files" not in output
assert "Bazel failed" in output
assert "| Repository | Policy | Status |" not in output