From dd13f53507afc4a22cd6a5f6136992ebfe9c7e01 Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:39:14 +0100 Subject: [PATCH] ci: support maintainer lab pull requests --- .github/pull_request_template.md | 12 +++-- .github/scripts/check-pr-description.js | 30 ++++++++++-- tests/test_pr_description_check.py | 62 +++++++++++++++++++++++-- 3 files changed, 91 insertions(+), 13 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index c54bf8963..a4fc0d5cd 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -4,12 +4,16 @@ ## Target branch -- [ ] This PR targets **`dev`**, not `main`. All PRs land in `dev`; `main` is curated by the maintainer at each release. If your PR is on `main` by accident, click "Edit" on this PR and change the base. +- [ ] This PR targets the correct integration branch: **`lab`** in the private maintainer-preview repository, or **`dev`** in the public repository. `main` remains release-curated. ## Linked Issue - + Fixes # @@ -25,7 +29,7 @@ Fixes # ## Checklist - [ ] I searched [open issues](https://github.com/odysseus-dev/odysseus/issues) and [open PRs](https://github.com/odysseus-dev/odysseus/pulls) — this is not a duplicate. -- [ ] This PR targets `dev` +- [ ] This PR targets the correct integration branch (`lab` in maintainer-preview; `dev` in the public repository) - [ ] My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in. - [ ] I actually ran the app (`docker compose up` or `uvicorn app:app`) and verified the change works end-to-end. Type-checks and unit tests are not enough. - [ ] I did not run the app/runtime validation and stated that gap in **How to Test**. Leave this unchecked when the app-run box above is checked. diff --git a/.github/scripts/check-pr-description.js b/.github/scripts/check-pr-description.js index d817d453a..3c0002a3e 100644 --- a/.github/scripts/check-pr-description.js +++ b/.github/scripts/check-pr-description.js @@ -8,6 +8,9 @@ module.exports = async ({ github, context, core }) => { const MARKER = ''; const owner = context.repo.owner; const repo = context.repo.repo; + const isMaintainerPreview = + owner === 'pewdiepie-archdaemon' + && repo === 'odysseus-maintainer-preview'; // Strip HTML comments so placeholder text does not count as content. function strip(text) { @@ -28,13 +31,30 @@ module.exports = async ({ github, context, core }) => { descriptionProblems.push('**Summary** is empty or too short — describe what changed and why.'); } - // 2. Linked Issue must reference a real issue. Accept a bare #NNN, a closing - // keyword + #NNN, or a full issue URL (e.g. .../issues/123) — the strict - // keyword-prefixed form previously false-flagged correctly-linked PRs. + // 2. Public contributor PRs must reference a real issue. The private + // maintainer-preview repository may explicitly opt out for fast maintainer + // integration work while still requiring the section to state that intent. const linkedSection = section('Linked Issue'); const hasIssueRef = /#\d+\b/.test(linkedSection) || /\/issues\/\d+/.test(linkedSection); - if (!linkedSection || !hasIssueRef) { - descriptionProblems.push('**Linked Issue** — add a reference like `Fixes #NNN`, a bare `#NNN`, or a link to the issue.'); + const hasMaintainerNA = /^N\/A\b/i.test(linkedSection); + + if (!linkedSection) { + descriptionProblems.push( + '**Linked Issue** — fill this section. Public PRs require an issue reference; ' + + 'maintainer-preview PRs may use `N/A — maintainer integration work`.' + ); + } else if (isMaintainerPreview) { + if (!hasIssueRef && !hasMaintainerNA) { + descriptionProblems.push( + '**Linked Issue** — use an issue reference or `N/A — maintainer integration work` ' + + 'in the private maintainer-preview repository.' + ); + } + } else if (!hasIssueRef) { + descriptionProblems.push( + '**Linked Issue** — add a reference like `Fixes #NNN`, a bare `#NNN`, ' + + 'or a link to the issue.' + ); } // 3. At least one Type of Change box must be checked. diff --git a/tests/test_pr_description_check.py b/tests/test_pr_description_check.py index 399095787..e2d1d81b4 100644 --- a/tests/test_pr_description_check.py +++ b/tests/test_pr_description_check.py @@ -14,14 +14,21 @@ _WORKFLOW = _REPO / ".github" / "workflows" / "pr-description-check.yml" pytestmark = pytest.mark.skipif(not shutil.which("node"), reason="node not on PATH") -def _body(*, app_ran=False, app_not_run=False, screenshot=False, media=""): +def _body( + *, + app_ran=False, + app_not_run=False, + screenshot=False, + media="", + linked_issue="Fixes #5934", +): return f"""## Summary This focused change has enough concrete summary detail for the checker. ## Linked Issue -Fixes #5934 +{linked_issue} ## Type of Change @@ -47,7 +54,15 @@ Run the focused checker regression tests and inspect their exact assertions. """ -def _run_checker(files, body, *, missing_labels=(), draft=False): +def _run_checker( + files, + body, + *, + missing_labels=(), + draft=False, + owner="odysseus-dev", + repo="odysseus", +): harness = r""" const checkPrDescription = require(process.argv[1]); const input = JSON.parse(process.argv[2]); @@ -89,7 +104,7 @@ const context = { draft: input.draft, }, }, - repo: { owner: 'odysseus-dev', repo: 'odysseus' }, + repo: { owner: input.owner, repo: input.repo }, }; const core = { warning: (message) => calls.push({ method: 'warning', message }), @@ -109,6 +124,8 @@ checkPrDescription({ github, context, core }) "body": body, "missingLabels": list(missing_labels), "draft": draft, + "owner": owner, + "repo": repo, } ) proc = subprocess.run( @@ -158,6 +175,43 @@ def test_complete_expected_state_is_ready(files, body): assert not any(call["method"] == "setFailed" for call in calls) +def test_public_repo_still_requires_linked_issue(): + calls = _run_checker( + ["README.md"], + _body(linked_issue="N/A — maintainer integration work"), + ) + + assert any(call["method"] == "setFailed" for call in calls) + assert "**Linked Issue**" in _comment(calls) + assert "ready for review" not in _added_labels(calls) + + +def test_maintainer_preview_accepts_explicit_na_linked_issue(): + calls = _run_checker( + ["README.md"], + _body(linked_issue="N/A — maintainer integration work"), + owner="pewdiepie-archdaemon", + repo="odysseus-maintainer-preview", + ) + + assert _added_labels(calls) == {"ready for review"} + assert not _comment(calls) + assert not any(call["method"] == "setFailed" for call in calls) + + +def test_maintainer_preview_rejects_ambiguous_non_issue_text(): + calls = _run_checker( + ["README.md"], + _body(linked_issue="No tracking needed"), + owner="pewdiepie-archdaemon", + repo="odysseus-maintainer-preview", + ) + + assert any(call["method"] == "setFailed" for call in calls) + assert "**Linked Issue**" in _comment(calls) + assert "ready for review" not in _added_labels(calls) + + def test_ui_checkbox_without_media_still_needs_visual_evidence(): calls = _run_checker( ["static/js/example.js"],