diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 0db002826..40c6896c8 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,47 +1,19 @@ -## Linked issue - -- Closes # -- Related # - -## Existing related work reviewed - -- Issues/PRs reviewed: -- If none found, write: None found after search - -## Overlap assessment - -- Classification: -- Overlapping items: -- Why this is not duplicate/superseded: - -## Why this PR should proceed - -- - ## Summary - What changed: - Why: -## Validation - -- [ ] Tests added/updated (or not applicable) -- [ ] Validation steps documented -- [ ] Evidence attached (logs/screenshots/output as relevant) - -## Documentation +## Linked issue (if applicable) -- [ ] Docs updated in this PR (or not applicable) -- [ ] Any setup/workflow changes reflected in repo docs +- Closes # -## Scope check +## Validation -- [ ] No unrelated refactors -- [ ] Implemented from a feature branch -- [ ] Change is deliverable without upstream `OSeMOSYS/MUIO` dependency -- [ ] Base repo/branch is `EAPD-DRB/MUIOGO:main` (not upstream) +- [ ] Tests added/updated (or not applicable) +- [ ] Manual verification steps documented, with evidence where relevant -## Exception rationale +## Checklist -- Only for narrow docs/typo PRs that do not use a linked issue. -- Leave blank otherwise. +- [ ] Change is scoped — no unrelated refactors +- [ ] Branched from `main`; PR targets `EAPD-DRB/MUIOGO:main` +- [ ] Docs updated for any setup, workflow, or architecture change diff --git a/.github/workflows/pr-intake.yml b/.github/workflows/pr-intake.yml deleted file mode 100644 index f3df54add..000000000 --- a/.github/workflows/pr-intake.yml +++ /dev/null @@ -1,84 +0,0 @@ -name: PR Intake - -on: - pull_request_target: - types: - - opened - - edited - - reopened - - synchronize - -permissions: - contents: read - pull-requests: write - issues: write - -jobs: - validate: - runs-on: ubuntu-latest - steps: - - name: Check out base repository - uses: actions/checkout@v4 - - - name: Check PR intake structure - id: intake - env: - GITHUB_TOKEN: ${{ github.token }} - run: | - set +e - python3 scripts/check_pr_intake.py \ - --repo "${{ github.repository }}" \ - --pr-number "${{ github.event.pull_request.number }}" \ - > intake_report.txt 2>&1 - status=$? - cat intake_report.txt - if [ "$status" -eq 0 ]; then - echo "needs_fix=false" >> "$GITHUB_OUTPUT" - else - echo "needs_fix=true" >> "$GITHUB_OUTPUT" - fi - - - name: Add intake label when structure is incomplete - if: steps.intake.outputs.needs_fix == 'true' - uses: actions/github-script@v7 - with: - script: | - try { - await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.payload.pull_request.number, - labels: ["needs-intake-fix"], - }); - } catch (error) { - console.log(`Warning: unable to add needs-intake-fix label: ${error.message}`); - } - - - name: Remove intake label when structure is complete - if: steps.intake.outputs.needs_fix != 'true' - uses: actions/github-script@v7 - with: - script: | - try { - await github.rest.issues.removeLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.payload.pull_request.number, - name: "needs-intake-fix", - }); - } catch (error) { - if (error.status !== 404) { - console.log(`Warning: unable to remove needs-intake-fix label: ${error.message}`); - } - } - - - name: Write intake summary - if: always() - run: | - if [ "${{ steps.intake.outputs.needs_fix }}" = "true" ]; then - echo "PR intake review: needs follow-up" >> "$GITHUB_STEP_SUMMARY" - echo "" >> "$GITHUB_STEP_SUMMARY" - cat intake_report.txt >> "$GITHUB_STEP_SUMMARY" - else - echo "PR intake review: structure looks complete" >> "$GITHUB_STEP_SUMMARY" - fi diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4bfd7f42c..c88b08f0e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,9 +7,8 @@ Thanks for contributing. 1. Read `README.md`, `docs/GSoC-2026.md`, `SUPPORT.md`, `docs/ARCHITECTURE.md`, and `docs/DOCS_POLICY.md` 2. Search existing issues and PRs before proposing implementation work 3. Create or reuse an issue before starting implementation work -4. Fill in the issue's required related-work fields so overlap is explicit -5. Create a feature branch from `main` -6. Confirm acceptance criteria in the issue so review can be objective +4. Create a feature branch from `main` +5. Confirm acceptance criteria in the issue so review can be objective ## Scope and repository boundaries @@ -33,36 +32,13 @@ Current tracks: - `Track: Integration` - `Track: Stability` -## Intake requirements +## Issues and PRs -Most implementation work must start from an issue. - -The issue must document: -- `Related issues reviewed` -- `Related PRs reviewed` -- `Overlap classification` -- `Why this issue is still needed` -- `Proposed track` - -If you found no relevant existing work, write `None found after search`. - -If overlapping work exists, explain why your issue is still needed and classify the overlap as one of: -- `none` -- `duplicate` -- `alternative approach` -- `complementary follow-up` -- `narrower fix` -- `superseding` - -If no overlap exists, a short current justification is enough. - -PRs should document: -- linked issue -- related work reviewed -- overlap assessment -- why the PR should proceed - -The `pr-intake` workflow is advisory. If required structure is missing, it may apply the `needs-intake-fix` label so maintainers can follow up. +- Search existing issues and PRs first so we avoid duplicate work. +- For anything beyond a small fix, open an issue before implementation so scope and acceptance criteria are clear. +- Link the issue from your PR (`Closes #123`). +- Keep each PR scoped to one issue, or a tightly related set. +- If you find related work, link it so any overlap is visible — a quick reference is enough. ## Workflow @@ -103,23 +79,7 @@ Post updates when one of these events occurs: - No unrelated refactors in the same PR - PR target is `EAPD-DRB/MUIOGO:main` (not upstream `OSeMOSYS/MUIO`) -### Narrow docs and typo exception - -For small docs or typo-only PRs, a linked issue may be skipped if the PR qualifies for the docs/typo exception. - -This exception is narrow. It does not cover: -- workflows -- issue or PR templates -- governance or policy files -- `CONTRIBUTING.md` - -Use the `Exception rationale` section in the PR template when claiming this exception. - -Docs-only PRs may still use a linked issue instead of the exception path. - -## Transition note - -Existing open PRs and older linked issues from before the intake rollout are transitional and may be handled manually while the new intake guidance is phased in. +Small docs or typo-only PRs do not need a linked issue. ## Definition of done diff --git a/docs/MAINTAINER_TRIAGE.md b/docs/MAINTAINER_TRIAGE.md index 4a5933ca7..db5e397cd 100644 --- a/docs/MAINTAINER_TRIAGE.md +++ b/docs/MAINTAINER_TRIAGE.md @@ -1,55 +1,17 @@ # Maintainer Triage Guide -This document defines the contribution-intake workflow introduced for issue-first, overlap-aware implementation work. +This document defines the triage vocabulary used on the project board — priorities, tracks, and stability lanes. These are maintainer-assigned metadata to organize work; they are not gates on contribution. -## Purpose +## Priorities -The intake system exists to reduce duplicate work, clarify scope before code is written, and keep review attention on the highest-signal items. +- `Priority: High` — work on ASAP +- `Priority: Medium` — important +- `Priority: Low` — may matter, can wait -Automation checks structure only: -- linked issue exists -- required issue sections are present -- required PR sections are present - -If structure is missing, automation may apply the `needs-intake-fix` label to the PR. - -Maintainers still decide whether the proposal is correct, useful, or merge-ready. - -## What to review on new implementation issues - -Confirm that the issue documents: -- `Related issues reviewed` -- `Related PRs reviewed` -- `Overlap classification` -- `Why this issue is still needed` -- `Proposed track` - -If related-work sections are weak but present, request clarification rather than bypassing the structure. - -If the issue is clearly duplicate or superseded, close or consolidate it rather than carrying parallel implementation work. - -## Overlap classifications - -Use these overlap terms consistently: -- `none`: no relevant overlapping work found -- `duplicate`: same problem and same intended fix as an existing issue or PR -- `alternative approach`: same problem, materially different solution path -- `complementary follow-up`: separate work that depends on or extends existing work -- `narrower fix`: intentionally smaller or more targeted than a broader open item -- `superseding`: intended to replace an older issue or PR as the canonical path - -## Handling overlap - -- Duplicate: point contributors to the canonical issue or PR and close the duplicate -- Alternative approach: keep only if there is a real design choice to evaluate -- Complementary follow-up: keep separate, but explicitly link the dependency chain -- Narrower fix: allow if the narrower path is more actionable than the broader one -- Superseding: make the replacement explicit in comments and close the older item when appropriate +Priority labels sync to the board's `Priority` field automatically (see `.github/workflows/sync-project-priority-from-labels.yml`). ## Tracks -Tracks are maintainer-assigned triage metadata. - - `Track: Cross-Platform` Cross-platform install, startup, path, and runtime compatibility work @@ -64,7 +26,7 @@ Tracks are maintainer-assigned triage metadata. ## Stability lanes -For now, Stability lanes are an internal triage vocabulary only. +Internal triage vocabulary for Stability-track work. - `Safety Guardrails` Narrow fixes that make the current synchronous design safer without redesigning execution flow @@ -75,11 +37,6 @@ For now, Stability lanes are an internal triage vocabulary only. - `Supporting Infrastructure` Run identity, atomic status tracking, shared metadata safety, and run-level observability work -## Transitional policy - -Open PRs that predate the intake rollout are transitional: -- the new `pr-intake` workflow may run on them -- maintainers may clean them up manually -- they are not retroactively required to match the new structure before the rollout is complete +## Handling duplicates and overlap -Once the queue is cleaned up, new or substantially updated PRs should follow the full intake format. The intake workflow is advisory and does not replace maintainer judgment. +If an issue or PR clearly duplicates or supersedes existing work, consolidate rather than carrying parallel work: point to the canonical item and close the duplicate, or link the dependency explicitly. Maintainers decide whether a proposal is correct, useful, or merge-ready. diff --git a/scripts/check_pr_intake.py b/scripts/check_pr_intake.py deleted file mode 100644 index 2a8beea31..000000000 --- a/scripts/check_pr_intake.py +++ /dev/null @@ -1,358 +0,0 @@ -#!/usr/bin/env python3 -"""Validate PR intake structure against repository contribution policy.""" - -from __future__ import annotations - -import argparse -import json -import os -import re -import sys -import urllib.error -import urllib.parse -import urllib.request -from typing import Iterable - - -ALLOWED_CLASSIFICATIONS = { - "none", - "duplicate", - "alternative approach", - "complementary follow-up", - "narrower fix", - "superseding", -} - -ALLOWED_TRACKS = { - "track: cross-platform", - "track: og onboarding", - "track: integration", - "track: stability", -} - -DOCS_ONLY_ALLOWED_EXACT = { - "README.md", - "SUPPORT.md", - "SECURITY.md", - "CODE_OF_CONDUCT.md", -} -DOCS_ONLY_ALLOWED_PREFIXES = ("docs/",) - -DOCS_ONLY_DISALLOWED_EXACT = { - "CONTRIBUTING.md", - "docs/MAINTAINER_TRIAGE.md", -} -DOCS_ONLY_DISALLOWED_PREFIXES = (".github/",) - -PLACEHOLDER_VALUES = { - "", - "-", - "tbd", - "n/a", - "none", -} - -HEADING_RE = re.compile(r"^\s{0,3}#{1,6}\s+(.+?)\s*$") -REF_RE = re.compile(r"#(\d+)") -NONE_FOUND_RE = re.compile(r"\bnone found after search\b", re.IGNORECASE) - - -class ValidationError(Exception): - """Raised when the PR intake structure is invalid.""" - - -def api_get(url: str, token: str) -> tuple[object, dict[str, str]]: - req = urllib.request.Request( - url, - headers={ - "Accept": "application/vnd.github+json", - "Authorization": f"Bearer {token}", - "X-GitHub-Api-Version": "2022-11-28", - "User-Agent": "muiogo-pr-intake", - }, - ) - try: - with urllib.request.urlopen(req) as response: - data = json.loads(response.read().decode("utf-8")) - headers = {k: v for k, v in response.headers.items()} - return data, headers - except urllib.error.HTTPError as exc: - detail = exc.read().decode("utf-8", "replace") - raise ValidationError(f"GitHub API request failed for {url}: {exc.code} {detail}") from exc - - -def parse_next_link(link_header: str | None) -> str | None: - if not link_header: - return None - for part in link_header.split(","): - match = re.search(r"<([^>]+)>;\s*rel=\"([^\"]+)\"", part) - if match and match.group(2) == "next": - return match.group(1) - return None - - -def fetch_paginated(url: str, token: str) -> list[object]: - items: list[object] = [] - next_url: str | None = url - while next_url: - data, headers = api_get(next_url, token) - if not isinstance(data, list): - raise ValidationError(f"Expected a list from GitHub API for {next_url}") - items.extend(data) - next_url = parse_next_link(headers.get("Link")) - return items - - -def extract_section(body: str, heading: str) -> str | None: - lines = body.splitlines() - target = heading.strip().lower() - - for idx, line in enumerate(lines): - match = HEADING_RE.match(line) - if match and match.group(1).strip().lower() == target: - collected: list[str] = [] - for inner in lines[idx + 1 :]: - if HEADING_RE.match(inner): - break - collected.append(inner) - return "\n".join(collected).strip() - return None - - -def cleaned_lines(text: str) -> list[str]: - return [line.strip() for line in text.splitlines() if line.strip()] - - -def normalize_text(text: str) -> str: - return re.sub(r"\s+", " ", text.strip()).strip().lower() - - -def looks_blank(text: str) -> bool: - normalized = normalize_text(text) - return normalized in PLACEHOLDER_VALUES - - -def path_is_docs_only_allowed(path: str) -> bool: - if path in DOCS_ONLY_DISALLOWED_EXACT: - return False - if any(path.startswith(prefix) for prefix in DOCS_ONLY_DISALLOWED_PREFIXES): - return False - if path in DOCS_ONLY_ALLOWED_EXACT: - return True - return any(path.startswith(prefix) for prefix in DOCS_ONLY_ALLOWED_PREFIXES) - - -def is_docs_exception_eligible(paths: Iterable[str]) -> bool: - path_list = list(paths) - return bool(path_list) and all(path_is_docs_only_allowed(path) for path in path_list) - - -def require_section(body: str, heading: str, errors: list[str], source: str) -> str | None: - section = extract_section(body, heading) - if section is None: - errors.append(f"{source} is missing required section: {heading}") - return None - if not cleaned_lines(section): - errors.append(f"{source} section is empty: {heading}") - return None - return section - - -def validate_issue_related_work(section: str, heading: str, errors: list[str]) -> None: - if REF_RE.search(section) or NONE_FOUND_RE.search(section): - return - errors.append( - f"Issue section '{heading}' must include issue/PR references like #123 or the phrase 'None found after search'" - ) - - -def validate_issue_overlap(section: str, errors: list[str]) -> None: - normalized = normalize_text(section) - if normalized not in ALLOWED_CLASSIFICATIONS: - errors.append( - "Issue section 'Overlap classification' must be one of: " - + ", ".join(sorted(ALLOWED_CLASSIFICATIONS)) - ) - - -def validate_issue_track(section: str, errors: list[str]) -> None: - normalized = normalize_text(section) - if normalized not in ALLOWED_TRACKS: - errors.append( - "Issue section 'Proposed track' must be one of: " - + ", ".join(sorted(ALLOWED_TRACKS)) - ) - - -def validate_issue_reason(section: str, heading: str, errors: list[str]) -> None: - if looks_blank(section) or len(normalize_text(section)) < 10: - errors.append(f"Issue section '{heading}' must contain a real explanation") - - -def find_linked_issue_numbers(section: str) -> list[int]: - return [int(match) for match in REF_RE.findall(section)] - - -def select_linked_issue(repo: str, candidates: list[int], token: str) -> tuple[int, dict[str, object]]: - for number in candidates: - issue, _headers = api_get(f"https://api.github.com/repos/{repo}/issues/{number}", token) - if not isinstance(issue, dict): - continue - if "pull_request" in issue: - continue - return number, issue - raise ValidationError( - "Linked issue section did not contain a valid issue reference. " - "Issue references must point to issues, not PRs." - ) - - -def validate_pr_related_work(section: str, errors: list[str]) -> None: - if REF_RE.search(section) or NONE_FOUND_RE.search(section): - return - errors.append( - "PR section 'Existing related work reviewed' must include issue/PR references like #123 or the phrase 'None found after search'" - ) - - -def validate_pr_overlap(section: str, errors: list[str]) -> None: - normalized = normalize_text(section) - has_classification = any(cls in normalized for cls in ALLOWED_CLASSIFICATIONS) - if not has_classification: - errors.append( - "PR section 'Overlap assessment' must include one overlap classification: " - + ", ".join(sorted(ALLOWED_CLASSIFICATIONS)) - ) - return - - explanation_lines = [] - for line in cleaned_lines(section): - if ":" in line: - _prefix, suffix = line.split(":", 1) - if suffix.strip(): - explanation_lines.append(suffix.strip()) - elif not line.startswith("-"): - explanation_lines.append(line) - - meaningful = [line for line in explanation_lines if normalize_text(line) not in PLACEHOLDER_VALUES] - if not meaningful or max(len(normalize_text(line)) for line in meaningful) < 8: - errors.append("PR section 'Overlap assessment' must include real overlap details, not just placeholders") - - -def validate_pr_reason(section: str, errors: list[str]) -> None: - if looks_blank(section) or len(normalize_text(section)) < 10: - errors.append("PR section 'Why this PR should proceed' must contain a real explanation") - - -def validate_exception_rationale(body: str, errors: list[str]) -> None: - section = extract_section(body, "Exception rationale") - if section is None: - errors.append("Docs/typo exception PRs must include the 'Exception rationale' section") - return - if looks_blank(section) or len(normalize_text(section)) < 10: - errors.append("Docs/typo exception PRs must provide a non-empty exception rationale") - - -def validate_issue_body(issue_body: str, errors: list[str]) -> None: - if not cleaned_lines(issue_body): - errors.append("Linked issue body must not be empty") - return - - related_issues = extract_section(issue_body, "Related issues reviewed") - related_prs = extract_section(issue_body, "Related PRs reviewed") - overlap = extract_section(issue_body, "Overlap classification") - still_needed = extract_section(issue_body, "Why this issue is still needed") - proposed_track = extract_section(issue_body, "Proposed track") - - if related_issues and not looks_blank(related_issues): - validate_issue_related_work(related_issues, "Related issues reviewed", errors) - if related_prs and not looks_blank(related_prs): - validate_issue_related_work(related_prs, "Related PRs reviewed", errors) - if overlap and not looks_blank(overlap): - validate_issue_overlap(overlap, errors) - if still_needed and not looks_blank(still_needed): - validate_issue_reason(still_needed, "Why this issue is still needed", errors) - if proposed_track and not looks_blank(proposed_track): - validate_issue_track(proposed_track, errors) - - -def validate_pr_body(repo: str, pr_body: str, token: str, errors: list[str]) -> None: - linked_issue = require_section(pr_body, "Linked issue", errors, "PR") - related_work = require_section(pr_body, "Existing related work reviewed", errors, "PR") - overlap = require_section(pr_body, "Overlap assessment", errors, "PR") - why_proceed = require_section(pr_body, "Why this PR should proceed", errors, "PR") - - if linked_issue is None: - return - - candidates = find_linked_issue_numbers(linked_issue) - if not candidates: - errors.append("PR section 'Linked issue' must include at least one issue reference such as Closes #123") - return - - try: - _issue_number, issue = select_linked_issue(repo, candidates, token) - except ValidationError as exc: - errors.append(str(exc)) - return - - issue_body = issue.get("body") or "" - validate_issue_body(issue_body, errors) - - if related_work is not None: - validate_pr_related_work(related_work, errors) - if overlap is not None: - validate_pr_overlap(overlap, errors) - if why_proceed is not None: - validate_pr_reason(why_proceed, errors) - - -def pr_has_linked_issue_reference(pr_body: str) -> bool: - linked_issue = extract_section(pr_body, "Linked issue") - if linked_issue is None: - return False - return bool(find_linked_issue_numbers(linked_issue)) - - -def main() -> int: - parser = argparse.ArgumentParser(description="Validate PR intake structure") - parser.add_argument("--repo", required=True, help="GitHub repository in owner/name form") - parser.add_argument("--pr-number", required=True, type=int, help="Pull request number") - args = parser.parse_args() - - token = os.environ.get("GITHUB_TOKEN", "").strip() - if not token: - print("FAIL: GITHUB_TOKEN is required") - return 1 - - pr_url = f"https://api.github.com/repos/{args.repo}/pulls/{args.pr_number}" - pr, _headers = api_get(pr_url, token) - if not isinstance(pr, dict): - print(f"FAIL: Unexpected PR payload from {pr_url}") - return 1 - - files_url = f"https://api.github.com/repos/{args.repo}/pulls/{args.pr_number}/files?per_page=100" - files = fetch_paginated(files_url, token) - changed_paths = [item["filename"] for item in files if isinstance(item, dict) and "filename" in item] - - errors: list[str] = [] - pr_body = pr.get("body") or "" - - if pr_has_linked_issue_reference(pr_body): - validate_pr_body(args.repo, pr_body, token, errors) - elif is_docs_exception_eligible(changed_paths): - validate_exception_rationale(pr_body, errors) - else: - validate_pr_body(args.repo, pr_body, token, errors) - - if errors: - for error in errors: - print(f"FAIL: {error}") - return 1 - - print("PASS: PR intake structure is valid") - return 0 - - -if __name__ == "__main__": - sys.exit(main())