Skip to content
Merged

Q.a #35

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
25 changes: 13 additions & 12 deletions .github/workflows/pr-metadata.yml
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
31 changes: 8 additions & 23 deletions .github/workflows/pr-sync.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,50 +4,35 @@ 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
issues: write
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:
Expand Down
17 changes: 11 additions & 6 deletions project_setup/pr_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@

VALIDATION_MARKER = "<!-- project-setup-pr-validation -->"
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"),
Expand Down Expand Up @@ -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 []
Expand Down Expand Up @@ -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)]


Expand Down
34 changes: 31 additions & 3 deletions scripts/validation/validate_pr_body.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions tests/fixtures/pr-sync-main-live.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Temporary live smoke-test marker for PR Sync after promotion to main.
Issue: #25
42 changes: 35 additions & 7 deletions tests/test_pr_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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 #<issue-number>\n\n## Summary\n- <what changed and why>"
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 = []
Expand Down Expand Up @@ -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 }}",
Expand All @@ -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)
Expand Down
Loading