diff --git a/.github/workflows/pr-metadata.yml b/.github/workflows/pr-metadata.yml index 268e5f1..3c91be5 100644 --- a/.github/workflows/pr-metadata.yml +++ b/.github/workflows/pr-metadata.yml @@ -1,26 +1,31 @@ name: PR metadata validation on: - pull_request_target: - types: [opened, synchronize, reopened, edited, ready_for_review] + workflow_run: + workflows: ["PR Sync"] + types: [completed] permissions: contents: read issues: write + pull-requests: read concurrency: - group: pr-metadata-${{ github.event.pull_request.number }} + group: pr-metadata-${{ github.event.workflow_run.pull_requests[0].number || github.event.workflow_run.id }} cancel-in-progress: true jobs: validate-pr: - if: github.event.pull_request.draft == false + if: >- + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.event == 'pull_request_target' && + github.event.workflow_run.pull_requests[0].number != null runs-on: ubuntu-latest steps: - - name: Checkout trusted base commit + - name: Checkout trusted default branch uses: actions/checkout@v6 with: - ref: ${{ github.event.pull_request.base.sha }} + ref: ${{ github.event.repository.default_branch }} persist-credentials: false - name: Set up Python @@ -30,17 +35,13 @@ jobs: - name: Validate branch name and pull request metadata env: - PR_BODY: ${{ github.event.pull_request.body }} - HEAD_REF: ${{ github.event.pull_request.head.ref }} - BASE_REF: ${{ github.event.pull_request.base.ref }} REPOSITORY: ${{ github.repository }} - PR_NUMBER: ${{ github.event.pull_request.number }} + PR_NUMBER: ${{ github.event.workflow_run.pull_requests[0].number }} GITHUB_TOKEN: ${{ github.token }} run: | set -euo pipefail python scripts/validation/validate_pr_body.py \ - --branch "$HEAD_REF" \ - --base-branch "$BASE_REF" \ --repo "$REPOSITORY" \ --pr-number "$PR_NUMBER" \ + --skip-draft-or-closed \ --comment diff --git a/.github/workflows/pr-sync.yml b/.github/workflows/pr-sync.yml index 501d867..a3c11df 100644 --- a/.github/workflows/pr-sync.yml +++ b/.github/workflows/pr-sync.yml @@ -4,11 +4,13 @@ name: PR Sync "on": pull_request_target: types: + - opened + - synchronize + - reopened + - edited + - ready_for_review - converted_to_draft - closed - workflow_run: - workflows: ["PR metadata validation", "PR guardrails"] - types: [completed] permissions: contents: read @@ -16,38 +18,21 @@ permissions: pull-requests: write concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.event.workflow_run.pull_requests[0].number || github.ref || github.run_id }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true jobs: sync: if: >- - ( - github.event_name == 'pull_request_target' && - github.event.pull_request.head.repo.full_name == github.repository - ) || ( - github.event_name == 'workflow_run' && - github.event.workflow_run.conclusion == 'success' && - github.event.workflow_run.event == 'pull_request_target' && - github.event.workflow_run.head_repository.full_name == github.repository && - github.event.workflow_run.pull_requests[0].number != null - ) + github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest steps: - - name: Checkout trusted base commit for direct PR lifecycle events - if: github.event_name == 'pull_request_target' + - name: Checkout trusted base commit uses: actions/checkout@v6 with: ref: ${{ github.event.pull_request.base.sha }} persist-credentials: false - - name: Checkout trusted base branch after PR guardrails - if: github.event_name == 'workflow_run' - uses: actions/checkout@v6 - with: - ref: refs/heads/${{ github.event.workflow_run.pull_requests[0].base.ref }} - persist-credentials: false - - name: Set up Python uses: actions/setup-python@v6 with: diff --git a/project_setup/pr_validation.py b/project_setup/pr_validation.py index 331a5f0..29c25bd 100644 --- a/project_setup/pr_validation.py +++ b/project_setup/pr_validation.py @@ -9,6 +9,10 @@ VALIDATION_MARKER = "" BRANCH_PATTERN = re.compile(r"^(feat|fix|docs|refactor|test|hotfix|phase|task|chore|ci|release)/[a-z0-9._/-]+$") +PROMOTION_PATHS = { + ("develop", "Q.A"), + ("Q.A", "main"), +} REQUIRED_SECTIONS = ( ("linked issue", "Linked Issue"), ("milestone", "Milestone"), @@ -55,14 +59,13 @@ def meaningful(lines: list[str]) -> bool: return False +def is_promotion_pull_request(branch: str | None, base_branch: str | None = None) -> bool: + return ((branch or "").strip(), (base_branch or "").strip()) in PROMOTION_PATHS + + def validate_branch(branch: str | None, base_branch: str | None = None) -> list[ValidationFinding]: normalized = (branch or "").strip() - base = (base_branch or "").strip() - promotion_paths = { - ("develop", "Q.A"), - ("Q.A", "main"), - } - if (normalized, base) in promotion_paths: + if is_promotion_pull_request(normalized, base_branch): return [] if BRANCH_PATTERN.fullmatch(normalized.casefold()): return [] @@ -93,6 +96,8 @@ def validate_body(body: str | None) -> list[ValidationFinding]: def validate_pull_request(branch: str | None, body: str | None, base_branch: str | None = None) -> list[ValidationFinding]: + if is_promotion_pull_request(branch, base_branch): + return [] return [*validate_branch(branch, base_branch), *validate_body(body)] diff --git a/scripts/validation/validate_pr_body.py b/scripts/validation/validate_pr_body.py index 9cb7307..b8e93ac 100644 --- a/scripts/validation/validate_pr_body.py +++ b/scripts/validation/validate_pr_body.py @@ -8,13 +8,24 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[2])) -from project_setup.github import get_token, require_client +from project_setup.github import API_BASE, get_token, require_client from project_setup.pr_validation import upsert_validation_comment, validate_pull_request -def read_body(args: argparse.Namespace) -> str: +def read_pull_request(args: argparse.Namespace) -> dict | None: + if not args.repo or not args.pr_number or not get_token(): + return None + return require_client().request_json( + "GET", + f"{API_BASE}/repos/{args.repo}/pulls/{args.pr_number}", + ) + + +def read_body(args: argparse.Namespace, pull_request: dict | None = None) -> str: if args.file: return Path(args.file).read_text(encoding="utf-8") + if pull_request is not None: + return pull_request.get("body") or "" if args.repo and args.pr_number and get_token(): return require_client().get_issue(args.repo, args.pr_number).get("body") or "" if os.getenv("PR_BODY") is not None: @@ -32,9 +43,26 @@ def main() -> int: parser.add_argument("--repo") parser.add_argument("--pr-number", type=int) parser.add_argument("--comment", action="store_true") + parser.add_argument( + "--skip-draft-or-closed", + action="store_true", + help="Skip active metadata validation when the resolved pull request is draft or closed.", + ) args = parser.parse_args() - findings = validate_pull_request(args.branch, read_body(args), args.base_branch) + pull_request = read_pull_request(args) + if args.skip_draft_or_closed and pull_request is not None: + if pull_request.get("draft") or pull_request.get("state") != "open": + print("Skipping metadata validation for draft or closed pull request.") + return 0 + + branch = args.branch + base_branch = args.base_branch + if pull_request is not None: + branch = branch or (pull_request.get("head") or {}).get("ref") + base_branch = base_branch or (pull_request.get("base") or {}).get("ref") + + findings = validate_pull_request(branch, read_body(args, pull_request), base_branch) for finding in findings: print(f"{finding.section}: {finding.problem}", file=sys.stderr) print(f" Fix: {finding.fix}", file=sys.stderr) diff --git a/tests/fixtures/pr-sync-main-live.txt b/tests/fixtures/pr-sync-main-live.txt new file mode 100644 index 0000000..7a62f11 --- /dev/null +++ b/tests/fixtures/pr-sync-main-live.txt @@ -0,0 +1,2 @@ +Temporary live smoke-test marker for PR Sync after promotion to main. +Issue: #25 diff --git a/tests/test_pr_sync.py b/tests/test_pr_sync.py index d48fed3..c94db18 100644 --- a/tests/test_pr_sync.py +++ b/tests/test_pr_sync.py @@ -20,6 +20,7 @@ sync_pr_metadata, upsert_sync_comment, ) +from project_setup.pr_validation import validate_pull_request ROOT = Path(__file__).resolve().parents[1] @@ -215,6 +216,11 @@ def test_promotion_pull_request_is_skipped(self): self.assertEqual(result, 0) client.get_issue.assert_not_called() + def test_promotion_pull_request_skips_implementation_metadata_body(self): + placeholder_body = "## Linked Issue\n- Closes #\n\n## Summary\n- " + self.assertEqual(validate_pull_request("develop", placeholder_body, "Q.A"), []) + self.assertEqual(validate_pull_request("Q.A", placeholder_body, "main"), []) + def test_missing_linked_task_sets_sticky_failure_comment(self): client = Mock(spec=GitHubClient) client.list_issue_comments.return_value = [] @@ -263,22 +269,21 @@ def test_success_comment_is_updated_instead_of_duplicated(self): class PrSyncWorkflowContractTests(unittest.TestCase): - def test_workflow_uses_trusted_base_and_guardrail_completion(self): + def test_pr_sync_runs_directly_for_relevant_pr_lifecycle_events(self): text = (ROOT / ".github/workflows/pr-sync.yml").read_text(encoding="utf-8") for expected in ( "name: PR Sync", "pull_request_target:", + "- opened", + "- synchronize", + "- reopened", + "- edited", + "- ready_for_review", "- converted_to_draft", "- closed", - "workflow_run:", - '"PR metadata validation"', - '"PR guardrails"', - "github.event.workflow_run.conclusion == 'success'", "github.event.pull_request.head.repo.full_name == github.repository", - "github.event.workflow_run.head_repository.full_name == github.repository", "ref: ${{ github.event.pull_request.base.sha }}", - "ref: refs/heads/${{ github.event.workflow_run.pull_requests[0].base.ref }}", "persist-credentials: false", "PROJECT_SETUP_PAT: ${{ secrets.PROJECT_SETUP_PAT }}", "PROJECT_SETUP_PROJECT_NUMBER: ${{ vars.PROJECT_SETUP_PROJECT_NUMBER }}", @@ -287,9 +292,32 @@ def test_workflow_uses_trusted_base_and_guardrail_completion(self): with self.subTest(expected=expected): self.assertIn(expected, text) + self.assertNotIn("workflow_run:", text) + self.assertNotIn('workflows: ["PR metadata validation", "PR guardrails"]', text) self.assertNotIn("github.event.pull_request.head.sha", text) self.assertNotIn("refs/heads/${{ github.event.pull_request.head.ref }}", text) + def test_metadata_validation_depends_on_successful_pr_sync(self): + text = (ROOT / ".github/workflows/pr-metadata.yml").read_text(encoding="utf-8") + + for expected in ( + "name: PR metadata validation", + "workflow_run:", + 'workflows: ["PR Sync"]', + "github.event.workflow_run.conclusion == 'success'", + "github.event.workflow_run.event == 'pull_request_target'", + "github.event.workflow_run.pull_requests[0].number != null", + "pull-requests: read", + "ref: ${{ github.event.repository.default_branch }}", + "--skip-draft-or-closed", + "--pr-number \"$PR_NUMBER\"", + ): + with self.subTest(expected=expected): + self.assertIn(expected, text) + + self.assertNotIn("pull_request_target:", text) + self.assertNotIn("PR_BODY:", text) + def test_installer_distributes_pr_sync_workflow(self): text = (ROOT / "project_setup/installer.py").read_text(encoding="utf-8") self.assertIn('".github/workflows/pr-sync.yml"', text)