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: 8 additions & 5 deletions repo_policy_sync/docs/explanation/pull-request-safety.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,15 @@ branch-head marker prevents closure and leaves the PR open for human review.

## Generated content

The runtime template is [pull_request.md](../../templates/pull_request.md). It
contains the policy identity and description, the non-compliant files that
triggered the pull request, any satisfied applicability condition, and the
The packaged runtime template is [pull_request.md](../../templates/pull_request.md),
and repositories can select a validated custom template through the
`pull_request_template` configuration setting or `--pull-request-template`.
Templates render the policy identity and description, the non-compliant files
that triggered the pull request, any satisfied applicability condition, and the
concrete changed files. A change can include one operation-level rationale; it
is rendered as a nested bullet below that change. The template also states that
the pull request is generated and must be reviewed before merging.
is rendered as a nested bullet below that change. All supported placeholders,
including the ownership and branch-head markers, are required so customized
body text cannot disable PR discovery or safety checks.

Repository Policy Sync applies the `automation` and `repo-policy-sync` labels
after PR creation. Before creating a PR, it creates either label when it is
Expand Down
1 change: 1 addition & 0 deletions repo_policy_sync/docs/reference/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ are CLI-only.
| `--json-output PATH` | Also write the versioned JSON report to `PATH`. |
| `--markdown-output PATH` | Also write the Markdown report to `PATH`. |
| `--policy-dir PATH` | Local policy directory. Repeat to combine directories. Defaults to `./policies` in the current working directory when present. |
| `--pull-request-template PATH` | Pull-request body template. Relative paths are resolved from the current working directory and override `pull_request_template` from TOML. |
| `--exclude-policy NAME` | Exclude one local or bundled policy. Applied after any explicit `--policy` selection; repeat to exclude more than one. |
| `--recreate` | On `apply`, rebuild one existing policy-owned pull request from its repository's current default branch. Requires exactly one `--repo` selection, exactly one selected repository after pattern expansion, and exactly one `--policy`. |
| `--allow-dirty-pr`, `--no-allow-dirty-pr` | After the automatic formatting-fix retry, commit and push changes even if pre-commit still fails; create or keep the pull request as a draft and add a comment with the failure. |
Expand Down
25 changes: 25 additions & 0 deletions repo_policy_sync/docs/reference/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ recreate = false
allow_dirty_pr = false
quiet = false
cache_dir = ".cache/repo-policy-sync"
pull_request_template = "templates/repo-policy-sync-pull-request.md"
sync_workers = 4
policy_workers = 4
```
Expand All @@ -55,6 +56,7 @@ The TOML keys map to the corresponding CLI options as follows:
| `allow_dirty_pr` | `--allow-dirty-pr` / `--no-allow-dirty-pr` |
| `quiet` | `--quiet` / `--no-quiet` |
| `cache_dir` | `--cache-dir` |
| `pull_request_template` | `--pull-request-template` |
| `sync_workers` | `--sync-workers` |
| `policy_workers` | `--policy-workers` |

Expand All @@ -65,6 +67,29 @@ directory exists. Setting it to `[]` disables local policy directories.
corresponding TOML value, including list-valued options. Unknown policy names
and unknown TOML fields are errors.

`pull_request_template` selects the body template for newly created and
updated policy pull requests. Relative TOML paths are resolved relative to the
configuration file. The command-line override resolves relative paths from the
current working directory. If omitted, the packaged
`repo_policy_sync/templates/pull_request.md` template is used.

Templates must contain each supported placeholder exactly by name; whitespace
inside the braces is allowed:

| Placeholder | Rendered value |
| --- | --- |
| `{{ policy_id }}` | Policy identifier. |
| `{{ policy_description }}` | Policy description, or the default description. |
| `{{ policy_trigger }}` | Why the repository matches the policy. |
| `{{ changes }}` | Markdown list of changed files and rationales. |
| `{{ failure_section }}` | Automation failure details, or empty text. |
| `{{ policy_marker }}` | Ownership marker used to identify the policy PR. |
| `{{ policy_head_marker }}` | Branch-head marker used by PR safety checks. |

Missing or unknown placeholders are configuration errors. The ownership and
head markers are mandatory because policy pull-request discovery and safety
checks depend on them.

Values in `repos` use the same repository selection rules as `--repo`: values
without `*`, `?`, or `[` are exact names, while values containing those
characters use case-sensitive Python `fnmatch` semantics. For example,
Expand Down
17 changes: 16 additions & 1 deletion repo_policy_sync/src/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

from .config import load_config
from .errors import PolicyError, RepoPolicySyncError
from .github import GitHubCli
from .github import GitHubCli, load_pull_request_template
from .policy import (
BUNDLED_POLICY_DIRECTORY,
DEFAULT_POLICY_DIRECTORY,
Expand Down Expand Up @@ -123,6 +123,14 @@ def _add_common_arguments(
action="append",
help="Local policy directory; repeat to combine directories (default: ./policies if present).",
)
rare.add_argument(
"--pull-request-template",
type=Path,
help=(
"Pull-request body template; relative paths are resolved from the "
"current working directory and override the TOML configuration."
),
)
rare.add_argument(
"--exclude-policy",
action="append",
Expand Down Expand Up @@ -203,6 +211,12 @@ def main(argv: Sequence[str] | None = None) -> int:
if args.cache_dir is not None
else (config.cache_directory or default_cache_directory())
)
pull_request_template_path = (
args.pull_request_template
if args.pull_request_template is not None
else config.pull_request_template
)
pull_request_template = load_pull_request_template(pull_request_template_path)
sync_workers = (
args.sync_workers
if args.sync_workers is not None
Expand Down Expand Up @@ -286,6 +300,7 @@ def main(argv: Sequence[str] | None = None) -> int:
apply=applying,
recreate=recreate,
allow_dirty_pr=allow_dirty_pr,
pull_request_template=pull_request_template,
sync_workers=sync_workers,
policy_workers=policy_workers,
# apply already looks up an open PR itself whenever it might act on
Expand Down
13 changes: 13 additions & 0 deletions repo_policy_sync/src/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class PolicySyncConfig:
allow_dirty_pr: bool | None = None
quiet: bool | None = None
cache_directory: Path | None = None
pull_request_template: Path | None = None
sync_workers: int | None = None
policy_workers: int | None = None

Expand Down Expand Up @@ -93,6 +94,7 @@ def load_config(path: Path | None = None) -> PolicySyncConfig:
"allow_dirty_pr",
"quiet",
"cache_dir",
"pull_request_template",
"sync_workers",
"policy_workers",
}
Expand Down Expand Up @@ -130,6 +132,16 @@ def load_config(path: Path | None = None) -> PolicySyncConfig:
config_path,
_string_value(section["cache_dir"], "cache_dir", config_path),
)
pull_request_template = None
if "pull_request_template" in section:
pull_request_template = _resolve_path(
config_path,
_string_value(
section["pull_request_template"],
"pull_request_template",
config_path,
),
)
sync_workers = _optional_positive_int(section, "sync_workers", config_path)
policy_workers = _optional_positive_int(section, "policy_workers", config_path)
return PolicySyncConfig(
Expand All @@ -142,6 +154,7 @@ def load_config(path: Path | None = None) -> PolicySyncConfig:
allow_dirty_pr=allow_dirty_pr,
quiet=quiet,
cache_directory=cache_directory,
pull_request_template=pull_request_template,
sync_workers=sync_workers,
policy_workers=policy_workers,
)
Expand Down
96 changes: 88 additions & 8 deletions repo_policy_sync/src/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from importlib.resources import files
from pathlib import Path

from .errors import CommandError, redact_sensitive_text
from .errors import CommandError, RepoPolicySyncError, redact_sensitive_text
from .models import Change, Policy, policy_branch_slug

TOOL_SLUG = "repo-policy-sync"
Expand All @@ -46,6 +46,17 @@
"USER",
"LOGNAME",
}
PULL_REQUEST_TEMPLATE_PLACEHOLDERS = (
"policy_id",
"policy_description",
"policy_trigger",
"changes",
"tool_revision",
"failure_section",
"policy_marker",
"policy_head_marker",
)
_PULL_REQUEST_TEMPLATE_PLACEHOLDER = re.compile(r"\{\{([^{}]*)\}\}")


@dataclass(frozen=True)
Expand Down Expand Up @@ -514,6 +525,7 @@ def create_pull_request(
head_oid: str,
draft: bool = False,
tool_revision: str,
pull_request_template: str | None = None,
) -> PullRequest:
self._ensure_automation_labels(repository=repository)
create_command = [
Expand All @@ -534,6 +546,7 @@ def create_pull_request(
changes,
head_oid=head_oid,
tool_revision=tool_revision,
pull_request_template=pull_request_template,
),
]
if draft:
Expand Down Expand Up @@ -649,6 +662,7 @@ def update_pull_request(
head_oid: str,
failure: str | None = None,
tool_revision: str,
pull_request_template: str | None = None,
) -> None:
"""Keep an existing policy-owned pull request's explanation current."""

Expand All @@ -665,7 +679,16 @@ def update_pull_request(
"-f",
f"title={policy.title}",
"-f",
f"body={_pull_request_body(policy, changes, head_oid=head_oid, failure=failure, tool_revision=tool_revision)}",
f"body={
_pull_request_body(
policy,
changes,
head_oid=head_oid,
failure=failure,
tool_revision=tool_revision,
pull_request_template=pull_request_template,
)
}",
]
)

Expand Down Expand Up @@ -839,8 +862,9 @@ def _pull_request_body(
head_oid: str,
tool_revision: str,
failure: str | None = None,
pull_request_template: str | None = None,
) -> str:
"""Build the concise, policy-centred pull-request template."""
"""Build a policy pull-request body from the selected validated template."""

description = (
policy.description
Expand All @@ -851,11 +875,16 @@ def _pull_request_body(
+ (f"\n - {change.rationale}" if change.rationale else "")
for change in changes
)
template = (
files("repo_policy_sync")
.joinpath("templates/pull_request.md")
.read_text(encoding="utf-8")
)
if pull_request_template is None:
template = load_pull_request_template()
else:
_validate_pull_request_template(
pull_request_template, source="provided pull-request template"
)
template = _PULL_REQUEST_TEMPLATE_PLACEHOLDER.sub(
lambda match: f"{{{{ {match.group(1).strip()} }}}}",
pull_request_template,
)
values = {
"policy_marker": _policy_marker(policy.id),
"policy_head_marker": _policy_head_marker(head_oid),
Expand All @@ -871,6 +900,57 @@ def _pull_request_body(
return template


def load_pull_request_template(path: Path | None = None) -> str:
"""Read and validate a packaged or user-provided pull-request template."""

source = (
str(path) if path is not None else "repo_policy_sync/templates/pull_request.md"
)
try:
template = (
path.read_text(encoding="utf-8")
if path is not None
else files("repo_policy_sync")
.joinpath("templates/pull_request.md")
Comment thread
MaximilianSoerenPollak marked this conversation as resolved.
.read_text(encoding="utf-8")
)
except OSError as exc:
raise RepoPolicySyncError(
f"could not read pull-request template {source}: {exc}"
) from exc
except UnicodeError as exc:
raise RepoPolicySyncError(
f"could not decode pull-request template {source} as UTF-8: {exc}"
) from exc
_validate_pull_request_template(template, source=source)
return template


def _validate_pull_request_template(template: str, *, source: str) -> None:
"""Reject templates that cannot render complete, safe policy PR bodies."""

placeholders = tuple(
placeholder.strip()
for placeholder in _PULL_REQUEST_TEMPLATE_PLACEHOLDER.findall(template)
)
supported = set(PULL_REQUEST_TEMPLATE_PLACEHOLDERS)
unknown = sorted(set(placeholders) - supported)
missing = [
placeholder
for placeholder in PULL_REQUEST_TEMPLATE_PLACEHOLDERS
if placeholder not in placeholders
]
if unknown or missing:
problems = []
if missing:
problems.append(f"missing placeholders: {', '.join(missing)}")
if unknown:
problems.append(f"unknown placeholders: {', '.join(unknown)}")
raise RepoPolicySyncError(
f"invalid pull-request template {source}: {'; '.join(problems)}"
)


def _failure_section(failure: str | None) -> str:
if failure is None:
return ""
Expand Down
Loading