Skip to content

feat(repo-policy-sync): discover policy PRs by label instead of branch head - #13

Merged
AlexanderLanin merged 6 commits into
eclipse-score:mainfrom
etas-contrib:feat/repo-policy-sync-label-based-pr-discovery
Aug 28, 2026
Merged

AlexanderLanin merged 6 commits into
eclipse-score:mainfrom
etas-contrib:feat/repo-policy-sync-label-based-pr-discovery

Conversation

@AlexanderLanin

@AlexanderLanin AlexanderLanin commented Aug 28, 2026 •

Copy link
Copy Markdown
Member

Why

When a policy was renamed, or a repository still had a pull request from the tool's predecessor, repo-policy-sync couldn't find it. Depending on the run, that meant it either created a duplicate pull request next to the existing one, or silently left the existing one out of its reports — the operator had no way to tell that a matching PR already existed.

The cause: ownership lookup called gh pr list --head <branch> once per candidate branch name, so it only ever saw PRs sitting on the exact branch names the current policy config expected. Anything on a different branch — a renamed policy's old branch, or a predecessor tool's naming convention — was invisible, no matter what its body marker said.

What

Label-based discovery

_find_policy_pull_requests now queries once via gh pr list --label repo-policy-sync --state <state> across the whole repository instead of looping per branch, and matches ownership against the PR body's policy marker. This is both fewer API calls and independent of branch naming.

Legacy migration support

A LEGACY_TOOL_SLUG = "repo-sync" marker/label is recognized alongside the current repo-policy-sync one, so PRs created by the tool's predecessor keep being recognized as owned. Three bundled policies gained legacy_names entries recording their former IDs from that migration.

Terminal PR status reporting

The terminal table now shows the discovered policy PR state (open/merged/closed) and number, not only the JSON/Markdown reports. plan runs request this status even without --json-output/--markdown-output, since the terminal table is now the primary place it is surfaced. The table intentionally keeps the compliance/not-applicable label when the found PR state doesn't add actionable information (e.g. a stale open PR that apply would close anyway) — see test_render_table_keeps_compliance_status_for_mismatched_pull_request.

Changes

  • src/github.py — discover owned PRs by label instead of per-branch --head lookup; add legacy tool-slug marker matching.
  • src/runner.py — fetch the policy PR status before evaluating applicability so not-applicable outcomes can report a matching PR too.
  • src/reporting.py — render PR state/number in the terminal table; keep it out of the Actions column now that it has its own status treatment.
  • src/cli.py — request PR status for plan runs, not only when writing JSON/Markdown reports.
  • policies/*/policy.yml — record former policy IDs in legacy_names from the predecessor tool's naming.
  • src/operations/README.md — moved from repo_policy_sync/operations/ to sit next to the operation modules it documents.
  • README.md, docs/** — document label-based discovery and legacy marker support.
  • tests/** — cover label-based lookup, legacy marker matching, and the new terminal PR status rendering.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Switches policy PR discovery from branch-based lookup to label/body-marker matching, with legacy migration and terminal status support.

Changes:

  • Adds label-based PR discovery and legacy marker handling.
  • Surfaces PR status in plan output.
  • Updates policies, tests, and documentation.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
repo_policy_sync/src/github.py Implements label-based discovery and legacy markers.
repo_policy_sync/src/runner.py Fetches PR status before applicability evaluation.
repo_policy_sync/src/reporting.py Adds PR state and number to terminal statuses.
repo_policy_sync/src/cli.py Enables PR-status lookup for plan runs.
repo_policy_sync/src/operations/README.md Relocates the operations catalogue.
repo_policy_sync/README.md Documents discovery and migration behavior.
repo_policy_sync/policies/minimum-bazel-version/policy.yml Adds a legacy policy ID.
repo_policy_sync/policies/minimal-bazel-module-declaration/policy.yml Adds legacy policy IDs.
repo_policy_sync/policies/docs-as-code-gitignore/policy.yml Expands legacy policy IDs.
repo_policy_sync/docs/reference/policy-format.md Documents marker-based migration.
repo_policy_sync/docs/reference/cli.md Documents terminal PR reporting.
repo_policy_sync/docs/how-to/run-a-policy.md Updates plan-mode guidance.
repo_policy_sync/docs/explanation/pull-request-safety.md Explains label and marker ownership.
repo_policy_sync/docs/explanation/architecture.md Updates the ownership model.
repo_policy_sync/tests/test_github.py Tests label lookup and legacy markers.
repo_policy_sync/tests/test_runner.py Tests status for inapplicable policies.
repo_policy_sync/tests/test_reporting.py Tests terminal PR rendering.
repo_policy_sync/tests/test_cli.py Tests plan status lookup.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread repo_policy_sync/src/github.py
Comment thread repo_policy_sync/src/github.py
Comment thread repo_policy_sync/src/github.py Outdated
Comment thread repo_policy_sync/README.md
…h head

PR ownership lookup used `gh pr list --head <branch>` once per candidate
branch. This tied ownership to the branch-naming scheme, so a policy rename
or a previous tool's differently-named branches were invisible to lookup.

Query PRs once via the `repo-policy-sync` label, then match ownership by the
marker in the PR body. Add a legacy `repo-sync` marker/label lookup so PRs
created by the tool's predecessor are still recognized, and record former
policy IDs (`legacy_names`) for policies that were renamed during that
migration.

Also report the discovered policy PR state (open/merged/closed/number) in
the terminal table, not just in JSON/Markdown output, and request it for
`plan` runs so it is visible without writing a report file.

Move repo_policy_sync/operations/README.md to src/operations/ alongside the
operation modules it documents.
…tatus

The label-based PR lookup cannot see a PR that never got the tool label
(a maintainer's own PR on the expected branch, or a tool-created PR whose
label application silently failed), so it could push over it or open a
duplicate. Restore a per-branch collision check independent of the label
filter, and make applying the tool label in create_pull_request mandatory
instead of a swallowed warning, since that label is the only signal later
runs use to recognize ownership.

Also propagate the newly-tracked closed PR state through runner.py's
policy_pr_status/url helpers, which previously could only report
open/merged/none even though PolicyPullRequestStatus.closed and the
reporting/README text already assumed a "closed" value existed.

Update the README's first-version compatibility note, which still claimed
no historical policy-ID compatibility while the same PR had already added
legacy_names support for renamed policies.
@AlexanderLanin
AlexanderLanin force-pushed the feat/repo-policy-sync-label-based-pr-discovery branch from 28ec852 to 973ff1f Compare August 28, 2026 11:50
The test called run_policies without _install_fake_sync, so it hit the
real gh auth check via sync_org and only passed locally because gh was
already authenticated. CI has no GitHub auth, so the tests job failed
with "You are not logged into any GitHub hosts".

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 6 comments.

Suppressed comments (1)

repo_policy_sync/src/github.py:294

  • Skipping the branch when any owned PR uses it leaves a collision undetected when the same head branch has a second, unlabeled open PR (GitHub allows this when PRs target different base branches). A later apply can then force-push that shared branch and alter the maintainer's PR. Always perform the branch lookup and verify that every returned PR number belongs to the owned labeled set, rather than treating branch presence as sufficient ownership.
        for branch in branches:
            if branch in owned_branches:
                continue

Comment thread repo_policy_sync/docs/how-to/run-a-policy.md Outdated
Comment thread repo_policy_sync/README.md Outdated
Comment thread repo_policy_sync/docs/reference/cli.md Outdated
Comment thread repo_policy_sync/src/cli.py Outdated
Comment thread repo_policy_sync/src/github.py
Comment thread repo_policy_sync/src/github.py Outdated
gh pr list --state closed returns merged PRs too (GitHub's PR state
only distinguishes open/closed; merged is a separate flag). Querying
for "merged" and "closed" separately therefore duplicated the closed
scan and let a merged PR appear as PolicyPullRequestStatus.closed
whenever its closedAt happened to be set.

Query --state closed once and classify locally by mergedAt, which
fixes the misclassification and drops one gh pr list call per
policy/repository status check. Update the docs and the outdated
cli.py comment that described the terminal table's PR-status column
as unconditional instead of shown only when that status is actionable.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Comment thread repo_policy_sync/src/runner.py

@MaximilianSoerenPollak MaximilianSoerenPollak left a comment

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.

Tried in Call, works as intended.
Seems alright.

…y this run

Plan reporting for a not-applicable policy with a pre-existing closed PR
produced the same status/policy_pr_status combination as an apply run that
just closed that PR, so run_policies incremented pull_requests_closed during
plan too. Add a pull_request_closed_by_run flag set only where the run
actually calls close_pull_request, and aggregate on that instead of
inferring the action from status + policy_pr_status.
@AlexanderLanin
AlexanderLanin merged commit f17dca6 into eclipse-score:main Aug 28, 2026
3 checks passed
@AlexanderLanin
AlexanderLanin deleted the feat/repo-policy-sync-label-based-pr-discovery branch August 28, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants