diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 5e6146049e..344aa931ef 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -13,23 +13,33 @@ > You can help us by categorizing your pull request with labels. > We appreciate you working with us and will get to reviewing your contribution as soon as we can! -As always, thank you for the contribution! - -### \~\~After reading, delete this line and the above to use the template for your pull request\~\~ +> [!NOTE] +> **Automated PR Sentinel checks (required):** a linked issue, a Conventional Commit title, Conventional Commit +> messages on every commit (no `Co-authored-by:` trailers), no merge commits (rebase instead), and a completed +> **AI Disclosure**. PRs that stay non-compliant are **auto-closed 3 days** after the first Sentinel notice. +> See the [Contribution Workflow](https://github.com/thunderbird/thunderbird-android/blob/main/docs/contributing/contribution-workflow.md#-automated-pr-checks-pr-sentinel). +As always, thank you for the contribution! +# \~\~After reading, delete this line and the above to use the template for your pull request\~\~ +------------------------------------------------------------------------------------------------- ## Contribution Summary -Linked Issue/Ticket: -RFC / Technical Design (if applicable): +Linked Issue/Ticket: __ -#### Description +RFC / Technical Design (if applicable): __ + +### Description __ -#### Screen Shots +### Screenshots + +__ + +### Testing -__ +__ ## AI Disclosure @@ -41,11 +51,11 @@ Select **one** of the following (mandatory) ## Contribution Checklist -- [ ] I have read and affirm that my contribution adheres to [Mozillaโ€™s Community Participation Guidelines](https://www.mozilla.org/en-US/about/governance/policies/participation/) +- [ ] I have read, and I affirm that my contribution adheres to [Mozillaโ€™s Community Participation Guidelines](https://www.mozilla.org/en-US/about/governance/policies/participation/) - [ ] This contribution is in Kotlin where possible - [ ] This contribution does not use merge commits - [ ] This contribution adheres to the existing codestyle (run `gradlew spotlessCheck` to check and `gradlew spotlessApply` to format your source code; will be checked by CI). - [ ] This contribution does not break existing unit tests (run `gradlew testDebugUnitTest`; will be checked by CI). -- [ ] This contribution includes tests for any new functionality, and maintains tests for any updated functionality. +- [ ] This contribution includes tests for any new functionality and maintains tests for any updated functionality. - [ ] This contribution adheres to our [Engineering process](https://github.com/thunderbird/thunderbird-android/tree/main/docs/engineering) (RFC/Technical Design/ADR) -- [ ] This PR has a descriptive title and body that accurately outlines all changes made, and contains a reference to any issues that it fixes (e.g. _Closes #XXX_ or _Fixes #XXX_). +- [ ] This PR has a descriptive title and body that accurately outlines all changes made and contains a reference to any issues that it fixes (e.g. _Closes #XXX_ or _Fixes #XXX_). diff --git a/.github/workflows/pr-request-report-labels.yml b/.github/workflows/pr-request-report-labels.yml index 1fb80c6fe5..faa7ed0546 100644 --- a/.github/workflows/pr-request-report-labels.yml +++ b/.github/workflows/pr-request-report-labels.yml @@ -54,4 +54,4 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} IDENTIFIER: "" run: | - ./scripts/ci/manage-pr-comment.sh "$PR_NUMBER" "$IDENTIFIER" "" "valid" + ./scripts/ci/manage-pr-comment.sh "$PR_NUMBER" "$IDENTIFIER" "" "valid" "โœ… **Validation Passed**: All report and feature-flag labels are correctly set." diff --git a/.github/workflows/pr-sentinel-autoclose.yml b/.github/workflows/pr-sentinel-autoclose.yml new file mode 100644 index 0000000000..1a4c377683 --- /dev/null +++ b/.github/workflows/pr-sentinel-autoclose.yml @@ -0,0 +1,66 @@ +--- +name: PR - Sentinel Auto-Close + +on: + schedule: + - cron: "0 0 * * *" # 00:00 GMT/UTC daily + workflow_dispatch: + inputs: + dry_run: + description: "Log escalate/close actions without making changes." + type: boolean + default: false + +permissions: + contents: read + pull-requests: write + issues: write + +jobs: + autoclose: + name: Escalate and close stale non-compliant PRs + runs-on: ubuntu-latest + timeout-minutes: 15 + environment: botmobile + env: + GH_REPO: ${{ github.repository }} + steps: + - name: Checkout code + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + + - name: App token generate + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + if: ${{ vars.BOT_CLIENT_ID }} + id: app-token + with: + client-id: ${{ vars.BOT_CLIENT_ID }} + private-key: ${{ secrets.BOT_PRIVATE_KEY }} + + - name: Assert Sentinel label exists (fail loud) + env: + GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }} + run: | + set -euo pipefail + # shellcheck source=scripts/ci/pr-sentinel/lib.sh disable=SC1091 + source ./scripts/ci/pr-sentinel/lib.sh + require_label_needs_updates + + - name: Escalate / close labeled PRs + env: + GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }} + DRY_RUN_INPUT: ${{ github.event.inputs.dry_run }} + run: | + set -euo pipefail + args=() + if [[ "${DRY_RUN_INPUT:-false}" == "true" ]]; then + args+=(--dry-run) + fi + numbers="$(gh pr list --state open --label "pr-sentinel: needs updates" \ + --json number --jq 'map(.number) | join(" ")')" + if [[ -z "$numbers" ]]; then + echo "No labeled PRs; nothing to do." + exit 0 + fi + for pr in $numbers; do + ./scripts/ci/pr-sentinel/escalate-pr.sh "$pr" "${args[@]}" + done diff --git a/.github/workflows/pr-sentinel.yml b/.github/workflows/pr-sentinel.yml new file mode 100644 index 0000000000..747e316793 --- /dev/null +++ b/.github/workflows/pr-sentinel.yml @@ -0,0 +1,88 @@ +--- +name: PR - Sentinel Bot + +# Warning, this job runs on pull_request_target and therefore has access to issue content. +# Don't add any steps that act on external (PR-head) code. +on: + pull_request_target: + types: + - opened + - reopened + - synchronize + - edited + - ready_for_review + - converted_to_draft + branches: + - main + workflow_dispatch: + inputs: + pr_number: + description: "Single PR to evaluate; leave blank to evaluate all open PRs." + required: false + default: "" + dry_run: + description: "Log actions without posting comments, changing labels, or closing PRs." + type: boolean + default: false + +permissions: + contents: read + pull-requests: write + issues: write + +concurrency: + group: pr-sentinel-${{ github.event.pull_request.number || github.run_id }} + cancel-in-progress: true + +jobs: + sentinel: + name: PR Sentinel + runs-on: ubuntu-latest + timeout-minutes: 10 + environment: botmobile + env: + GH_REPO: ${{ github.repository }} + steps: + - name: Checkout code + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + + - name: App token generate + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + if: ${{ vars.BOT_CLIENT_ID }} + id: app-token + with: + client-id: ${{ vars.BOT_CLIENT_ID }} + private-key: ${{ secrets.BOT_PRIVATE_KEY }} + + - name: Determine target PRs + id: prs + env: + GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }} + EVENT_NAME: ${{ github.event_name }} + EVENT_PR: ${{ github.event.pull_request.number }} + INPUT_PR: ${{ github.event.inputs.pr_number }} + run: | + set -euo pipefail + if [[ "$EVENT_NAME" == "pull_request_target" ]]; then + numbers="$EVENT_PR" + elif [[ -n "$INPUT_PR" ]]; then + numbers="$INPUT_PR" + else + numbers="$(gh pr list --state open --json number --jq 'map(.number) | join(" ")')" + fi + echo "numbers=${numbers}" >> "$GITHUB_OUTPUT" + + - name: Evaluate and report + env: + GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }} + NUMBERS: ${{ steps.prs.outputs.numbers }} + DRY_RUN_INPUT: ${{ github.event.inputs.dry_run }} + run: | + set -euo pipefail + args=() + if [[ "${DRY_RUN_INPUT:-false}" == "true" ]]; then args+=(--dry-run); fi + overall=0 + for pr in $NUMBERS; do + ./scripts/ci/pr-sentinel/report-pr.sh "$pr" "${args[@]}" || overall=1 + done + exit "$overall" diff --git a/docs/contributing/contribution-workflow.md b/docs/contributing/contribution-workflow.md index 0911cdec57..d324e1c75a 100644 --- a/docs/contributing/contribution-workflow.md +++ b/docs/contributing/contribution-workflow.md @@ -213,10 +213,14 @@ To submit your changes for review: - Base branch: `main` - Head repo: your fork & branch 4. Select your fork and branch as the source -5. Click **Create pull request** +5. Make sure your [Pull Request Description](https://github.com/thunderbird/thunderbird-android/blob/main/docs/contributing-workflow.md#pull-request-description) + is compliant with our guidelines +6. Click **Create pull request** ### Pull Request Description +Before start writing the description, read the Pull Request template that will be prompted for you. + Write a clear and concise description for your pull request: 1. Reference the issue number (e.g., "Fixes #123", "Resolves #456") @@ -225,12 +229,29 @@ Write a clear and concise description for your pull request: 4. Include screenshots or videos for UI changes 5. Mention any related issues or pull requests -Example: +#### Mandatory fields + +When writing your Pull Request Description, you must fill all the mandatory fields: + +- **Linked Issue/Ticket** + - Must use closing keywords, e.g. Closes, Fixes, Resolves, etc. See [Linking a pull request to an issue using a keyword](https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword) + for available keywords. +- **Description** +- **AI Disclosure** + - Must check at least one of the checkboxes +- **Contribution Checklist** + - Must follow all the instructions of the checklist and then mark as completed by checking all. + +An automated PR Sentinel will be checking if you are following the requirements and will flag the PR if not. +See [Automated PR checks (PR Sentinel)](https://github.com/thunderbird/thunderbird-android/blob/main/docs/contributing/contribution-workflow.md#-automated-pr-checks-pr-sentinel) +for more details. + +#### Example ```markdown ## Contribution Summary -Linked Issue/Ticket: #123 +Linked Issue/Ticket: Closes #123 RFC / Technical Design (if applicable): #### Description @@ -240,9 +261,9 @@ This PR adds email validation to the login form. It: - Shows error messages for invalid emails - Adds unit tests for the validation logic -#### Screen Shots +#### Screenshots -[Screenshot of error message] +[Screenshot of the error message] #### Testing @@ -250,6 +271,26 @@ This PR adds email validation to the login form. It: 2. Verify that an error message appears 3. Enter a valid email 4. Verify that the error message disappears + +## AI Disclosure + +Select **one** of the following (mandatory) + +- [x] This contribution does not include any changes created or assisted by AI. +- [ ] This contribution includes changes assisted by AI. +- [ ] This contribution includes changes created by AI. + +## Contribution Checklist + +- [x] I have read, and I affirm that my contribution adheres to [Mozillaโ€™s Community Participation Guidelines](https://www.mozilla.org/en-US/about/governance/policies/participation/) +- [x] This contribution is in Kotlin where possible +- [x] This contribution does not use merge commits +- [x] This contribution adheres to the existing codestyle (run `gradlew spotlessCheck` to check and `gradlew spotlessApply` to format your source code; will be checked by CI). +- [x] This contribution does not break existing unit tests (run `gradlew testDebugUnitTest`; will be checked by CI). +- [x] This contribution includes tests for any new functionality, and maintains tests for any updated functionality. +- [x] This contribution adheres to our [Engineering process](https://github.com/thunderbird/thunderbird-android/tree/main/docs/engineering) (RFC/Technical Design/ADR) +- [x] This PR has a descriptive title and body that accurately outlines all changes made, and contains a reference to any issues that it fixes (e.g. _Closes #XXX_ or _Fixes #XXX_). + ``` ## ๐Ÿ‘€ Code Review Process @@ -265,6 +306,26 @@ After submitting your pull request: ๐Ÿ‘‰ For expectations and etiquette, see [Code Review Guide](code-review-guide.md). +### ๐Ÿค– Automated PR checks (PR Sentinel) + +Every PR is validated automatically to verify if the PR is ready for review. To be reviewable it must have: + +1. A **linked issue** in the description (e.g. `Closes #123`). +2. A **Conventional Commit title** (see the [Git Commit Guide](git-commit-guide.md)). +3. **Conventional Commit messages** on every commit, with **no `Co-authored-by:` trailers**. +4. A completed **AI Disclosure** section (exactly one option selected). +5. **No merge commits** โ€” rebase onto the base branch instead of merging. + +When something is missing, PR Sentinel posts a single comment listing it and labels the PR +`pr-sentinel: needs updates`. If it stays unresolved, the bot posts a closing warning after **1 day** +and **auto-closes the PR after 3 days**. + +Push a fix (or edit the description), and the comment is removed, the `pr-sentinel: needs updates` +label is swapped for **`pr-sentinel: ready for review`**, and the check turns green. + +**Draft PRs are skipped** until marked ready for review, and **bot PRs** (Dependabot, Renovate, โ€ฆ) are exempt. +Everyone else โ€” **including maintainers and members** โ€” must comply. + ## ๐Ÿ”„ Keeping Your Fork Updated To keep your fork in sync with the main repository: diff --git a/docs/contributing/git-commit-guide.md b/docs/contributing/git-commit-guide.md index 85dd3a6974..8cb5894cf1 100644 --- a/docs/contributing/git-commit-guide.md +++ b/docs/contributing/git-commit-guide.md @@ -5,6 +5,14 @@ This makes your work easier to review, track, and maintain for everyone involved ## โœ๏ธ Commit Message Format +> [!IMPORTANT] +> **Enforced in CI:** +> - the PR title **and every commit subject** must match this format (checked by the [PR Sentinel workflow](https://github.com/thunderbird/thunderbird-android/blob/main/.github/workflows/pr-sentinel.yml)). +> - The `` must be 70 characters or fewer. +> +> See [Automated PR checks (PR Sentinel)](https://github.com/thunderbird/thunderbird-android/blob/main/docs/contributing/contribution-workflow.md#-automated-pr-checks-pr-sentinel) +> for more details. + ```git (): @@ -17,7 +25,7 @@ Components: - ``: The [type of change](#-commit-types) being made (e.g., feat, fix, docs). - `` **(optional)**: The [scope](#optional-scope) indicates the area of the codebase affected by the change (e.g., auth, ui). -- ``: Short description of the change (50 characters or less) +- ``: Short description of the change (70 characters or less) - `` **(optional)**: Explain what changed and why, include context if helpful. - `` **(optional)**: Include issue references, breaking changes, etc. @@ -63,20 +71,21 @@ Fixes #123 The **scope** is optional but recommended for clarity, especially for large changes or or when multiple areas of the codebase are involved. -| Scope | Use for... | Example | -|------------|----------------|------------------------------------------| -| `auth` | Authentication | `feat(auth): add login functionality` | -| `settings` | User settings | `feat(settings): add dark mode toggle` | -| `build` | Build system | `fix(build): improve build performance` | -| `ui` | UI/theme | `refactor(ui): split theme into modules` | -| `deps` | Dependencies | `chore(deps): bump Kotlin to 2.0.0` | +| Scope | Use for... | Example | +|------------|------------------|------------------------------------------| +| `auth` | Authentication | `feat(auth): add login functionality` | +| `settings` | User settings | `feat(settings): add dark mode toggle` | +| `build` | Build system | `fix(build): improve build performance` | +| `ci` | CI configuration | `chore(ci): add PR Sentinel workflow` | +| `ui` | UI/theme | `refactor(ui): split theme into modules` | +| `deps` | Dependencies | `chore(deps): bump Kotlin to 2.0.0` | ## ๐Ÿง  Best Practices ### 1. One Commit, One Purpose - โœ… Each commit should represent a single logical change or addition to the codebase. -- โŒ Donโ€™t mix unrelated changes together (e.g., fixing a bug and updating docs, or changing a style and ) +- โŒ Donโ€™t mix unrelated changes together (e.g., fixing a bug and updating docs, or changing a style and adding a feature). ### 2. Keep It Manageable diff --git a/scripts/ci/manage-pr-comment.sh b/scripts/ci/manage-pr-comment.sh index 498aa0d071..9cfed536ff 100755 --- a/scripts/ci/manage-pr-comment.sh +++ b/scripts/ci/manage-pr-comment.sh @@ -4,10 +4,13 @@ set -euo pipefail # This script manages a single PR comment by using a unique identifier. # Usage: -# ./manage-pr-comment.sh +# ./manage-pr-comment.sh [resolved-message] +# +# resolved-message is optional and only used when status is "valid" and an +# existing comment is present. Defaults to a generic pass message. -if [[ $# -ne 4 ]]; then - echo "Usage: $0 " +if [[ $# -lt 4 || $# -gt 5 ]]; then + echo "Usage: $0 [resolved-message]" exit 1 fi @@ -15,6 +18,7 @@ PR_NUMBER="$1" IDENTIFIER="$2" MESSAGE="$3" STATUS="$4" +RESOLVED_MESSAGE_TEXT="${5:-โœ… **Validation Passed**}" COMMENT_ID=$(gh api "repos/{owner}/{repo}/issues/${PR_NUMBER}/comments" | \ jq -r ".[] | select(.body | contains(\"${IDENTIFIER}\")) | .id" | head -n 1) @@ -35,7 +39,7 @@ if [[ "$STATUS" == "invalid" ]]; then fi elif [[ "$STATUS" == "valid" ]]; then if [[ -n "$COMMENT_ID" ]]; then - RESOLVED_MESSAGE="โœ… **Validation Passed**: All report and feature-flag labels are correctly set.${IDENTIFIER}" + RESOLVED_MESSAGE="${RESOLVED_MESSAGE_TEXT}${IDENTIFIER}" echo "Marking comment $COMMENT_ID as resolved" gh api -X PATCH "repos/{owner}/{repo}/issues/comments/${COMMENT_ID}" -f body="$RESOLVED_MESSAGE" > /dev/null else diff --git a/scripts/ci/pr-sentinel/checks.sh b/scripts/ci/pr-sentinel/checks.sh new file mode 100755 index 0000000000..b5868fd320 --- /dev/null +++ b/scripts/ci/pr-sentinel/checks.sh @@ -0,0 +1,97 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Pure PR Sentinel check functions. No network, no side effects. +# Each check_* echoes "" on pass, or a single-line failure message. +# +# Backticks in the messages below are literal Markdown for the PR comment, not +# command substitution โ€” single quotes are intentional. +COMMIT_MESSAGE_TYPES_REGEX='feat|fix|docs|style|refactor|test|chore|revert' +COMMIT_MESSAGE_SCOPE_REGEX='\([a-zA-Z0-9._ -/:]+\)' + +check_title() { + local title="$1" + local pattern="^(${COMMIT_MESSAGE_TYPES_REGEX})(${COMMIT_MESSAGE_SCOPE_REGEX})?!?: .+$" + if [[ "$title" =~ $pattern ]]; then printf '' + else printf 'Title must follow Conventional Commits (e.g. `fix(ui): correct padding`).'; fi +} + +check_linked_issue() { + local body="$1" + # Strip code before matching: a closing keyword inside inline code (`Closes #1234`, + # e.g. the PR-template placeholder) or a fenced block does NOT link an issue on + # GitHub, so it must not count here either. + local stripped + stripped="$(printf '%s\n' "$body" | tr -d '\r' | awk ' + /^[[:space:]]*```/ { infence = !infence; next } # toggle fenced code blocks + infence { next } # drop fenced content + { gsub(/`[^`]*`/, ""); print } # drop inline code spans + ')" + local pattern='(^|[^[:alnum:]_])(close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved|part of) #[0-9]+' + local rc=1 + shopt -s nocasematch + if [[ "$stripped" =~ $pattern ]]; then rc=0; fi + shopt -u nocasematch + if [[ "$rc" -eq 0 ]]; then printf '' + else printf 'Link an issue in the description using a closing keyword (e.g. `Closes #123`).'; fi +} + +check_ai_disclosure() { + local body="$1" checked + checked="$(printf '%s\n' "$body" | tr -d '\r' | awk ' + /^##[[:space:]]+AI Disclosure/ { in_section = 1; next } + /^##[[:space:]]/ && in_section { in_section = 0 } + in_section { print } + ' | grep -cE '^[[:space:]]*[-*][[:space:]]+\[[xX]\]' || true)" + if [[ "$checked" -eq 1 ]]; then printf '' + elif [[ "$checked" -eq 0 ]]; then printf 'Complete the **AI Disclosure** section (tick exactly one box).' + else printf 'AI Disclosure has %s boxes ticked; tick exactly one.' "$checked"; fi +} + +check_commit_subject() { + local sha="$1" subject="$2" + # Group 3 captures the only (type and optional scope are excluded). + local pattern="^(${COMMIT_MESSAGE_TYPES_REGEX})(${COMMIT_MESSAGE_SCOPE_REGEX})?!?: (.+)$" + if [[ ! "$subject" =~ $pattern ]]; then + printf 'Commit %s: subject must follow Conventional Commits.' "$sha"; return + fi + local desc="${BASH_REMATCH[3]}" + MAX_COMMIT_DESC_SIZE=70 + if (( ${#desc} > MAX_COMMIT_DESC_SIZE )); then + printf 'Commit %s: description is %s chars (max %d).' "$sha" "${#desc}" "$MAX_COMMIT_DESC_SIZE"; return + fi + printf '' +} + +check_commit_coauthor() { + local sha="$1" message="$2" + if printf '%s\n' "$message" | grep -qiE '^Co-authored-by:'; then + printf 'Commit %s: remove the `Co-authored-by:` trailer.' "$sha" + else printf ''; fi +} + +is_exempt() { + # Only bot authors are exempt; everyone else (maintainers, drafts, contributors) + # must comply with the PR guidelines. + if [[ "$1" == "Bot" ]]; then printf 'bot'; fi +} + +decide_escalation() { + local age="$1" + if (( age >= 259200 )); then printf 'close' + elif (( age >= 86400 )); then printf 'escalate' + else printf 'none'; fi +} + +# Convert an ISO-8601 UTC timestamp (e.g. 2020-01-01T00:00:00Z) to epoch seconds. +# Works with both GNU date (-d, on CI/Linux) and BSD date (-j -f, on macOS). +iso_to_epoch() { + local iso="$1" + date -u -d "$iso" +%s 2>/dev/null || date -u -j -f '%Y-%m-%dT%H:%M:%SZ' "$iso" +%s 2>/dev/null +} + +# Format epoch seconds as a human-readable UTC timestamp. GNU/BSD compatible. +epoch_to_display() { + local epoch="$1" + date -u -d "@${epoch}" '+%Y-%m-%d %H:%M UTC' 2>/dev/null || date -u -r "${epoch}" '+%Y-%m-%d %H:%M UTC' 2>/dev/null +} diff --git a/scripts/ci/pr-sentinel/escalate-pr.sh b/scripts/ci/pr-sentinel/escalate-pr.sh new file mode 100755 index 0000000000..b3fe5ba303 --- /dev/null +++ b/scripts/ci/pr-sentinel/escalate-pr.sh @@ -0,0 +1,104 @@ +#!/usr/bin/env bash +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${SCRIPT_DIR}/lib.sh" +source "${SCRIPT_DIR}/checks.sh" + +usage() { + cat <<'USAGE' +Usage: escalate-pr.sh [--dry-run] [-h|--help] + +Escalate or close a non-compliant pull request based on how long its PR Sentinel status +comment has existed: escalate (add a close-countdown banner) after 1 day, close after 3 days. + +Arguments: + Pull request number to act on (required, numeric). + +Options: + --dry-run Log state-changing actions without executing them. + -h, --help Show this help and exit. + +Environment: + GH_TOKEN, GH_REPO Passed through to gh (required in CI). + DRY_RUN=1 Alternative to --dry-run. + NOW_EPOCH Override "now" (epoch seconds) for testing. +USAGE +} + +PR_NUMBER="" +while [[ $# -gt 0 ]]; do + case "$1" in + -h|--help) usage; exit 0 ;; + --dry-run) DRY_RUN=1 ;; + -*) echo "Unknown option: $1" >&2; usage >&2; exit 2 ;; + *) + if [[ -n "$PR_NUMBER" ]]; then echo "Unexpected argument: $1" >&2; usage >&2; exit 2; fi + PR_NUMBER="$1" + ;; + esac + shift +done + +if [[ -z "$PR_NUMBER" ]]; then + echo "Error: is required." >&2 + usage >&2 + exit 2 +fi + +if [[ ! "$PR_NUMBER" =~ ^[0-9]+$ ]]; then + echo "Error: must be numeric: $PR_NUMBER" >&2 + usage >&2 + exit 2 +fi +export DRY_RUN # read by lib.sh's gh_write + +NOW_EPOCH="${NOW_EPOCH:-$(date -u +%s)}" + +# Print an action summary, phrased for real vs dry-run. +# $1 = wording when the action ran; $2 = wording when it was only simulated. +summary() { + if is_dry_run; then echo "PR #${PR_NUMBER}: [dry-run] $2"; else echo "PR #${PR_NUMBER}: $1"; fi +} + +# Draft PRs are skipped (defensive; report-pr.sh normally unlabels them on draft). +pr_json="$(gh api "repos/{owner}/{repo}/pulls/${PR_NUMBER}")" +if [[ "$(jq -r '.draft' <<<"$pr_json")" == "true" ]]; then + echo "PR #${PR_NUMBER}: draft; skipping." + exit 0 +fi + +created_at="$(get_status_comment_created_at "$PR_NUMBER")" +if [[ -z "$created_at" ]]; then + echo "PR #${PR_NUMBER}: no status comment; skipping." + exit 0 +fi + +t0="$(iso_to_epoch "$created_at")" # GNU/BSD portable +age=$(( NOW_EPOCH - t0 )) +action="$(decide_escalation "$age")" +close_iso="$(epoch_to_display "$(( t0 + 259200 ))")" +author="$(jq -r '.user.login' <<<"$pr_json")" + +case "$action" in + none) + echo "PR #${PR_NUMBER}: age ${age}s (<24h); no action." + ;; + escalate) + body="$(get_status_comment_body "$PR_NUMBER")" + if grep -q "$PR_SENTINEL_ESCALATED_MARKER" <<<"$body"; then + echo "PR #${PR_NUMBER}: already escalated; not re-notifying." + else + banner="> โš ๏ธ @${author} โ€” this PR will be **auto-closed on ${close_iso}** if the items below are not resolved." + new_body="${banner}"$'\n\n'"${body}"$'\n'"${PR_SENTINEL_ESCALATED_MARKER}" + id="$(find_status_comment_id "$PR_NUMBER")" + gh_write api -X PATCH "repos/{owner}/{repo}/issues/comments/${id}" -f body="$new_body" + summary "escalated (close on ${close_iso})" "would escalate (close on ${close_iso})" + fi + ;; + close) + gh_write api -X POST "repos/{owner}/{repo}/issues/${PR_NUMBER}/comments" \ + -f body="๐Ÿค– @${author} โ€” closing this PR because the required items were not resolved within 3 days. Please address them and reopen. ${PR_SENTINEL_MARKER}" + gh_write pr close "$PR_NUMBER" + summary "closed" "would close" + ;; +esac diff --git a/scripts/ci/pr-sentinel/evaluate-pr.sh b/scripts/ci/pr-sentinel/evaluate-pr.sh new file mode 100755 index 0000000000..5a8acdbca3 --- /dev/null +++ b/scripts/ci/pr-sentinel/evaluate-pr.sh @@ -0,0 +1,95 @@ +#!/usr/bin/env bash +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${SCRIPT_DIR}/checks.sh" + +usage() { + cat <<'USAGE' +Usage: evaluate-pr.sh [-h|--help] + +Evaluate a pull request against the PR Sentinel mandatory checks and print a JSON +verdict to stdout: {compliant, exempt, exempt_reason, missing_markdown}. + +Arguments: + Pull request number to evaluate (required, numeric). + +Options: + -h, --help Show this help and exit. + +Environment: + GH_TOKEN, GH_REPO Passed through to gh. + PR_JSON, COMMITS_JSON Optional fixtures; when set, the API is not called (used by tests). +USAGE +} + +PR_NUMBER="" +while [[ $# -gt 0 ]]; do + case "$1" in + -h|--help) usage; exit 0 ;; + -*) echo "Unknown option: $1" >&2; usage >&2; exit 2 ;; + *) + if [[ -n "$PR_NUMBER" ]]; then echo "Unexpected argument: $1" >&2; usage >&2; exit 2; fi + PR_NUMBER="$1" + ;; + esac + shift +done +if [[ -z "$PR_NUMBER" ]]; then echo "Error: is required." >&2; usage >&2; exit 2; fi +if [[ ! "$PR_NUMBER" =~ ^[0-9]+$ ]]; then echo "Error: must be numeric: $PR_NUMBER" >&2; exit 2; fi + +PR_JSON="${PR_JSON:-$(gh api "repos/{owner}/{repo}/pulls/${PR_NUMBER}")}" +COMMITS_JSON="${COMMITS_JSON:-$(gh api --paginate "repos/{owner}/{repo}/pulls/${PR_NUMBER}/commits")}" + +title="$(jq -r '.title // ""' <<<"$PR_JSON")" +body="$(jq -r '.body // ""' <<<"$PR_JSON")" +author_type="$(jq -r '.user.type // ""' <<<"$PR_JSON")" +draft="$(jq -r '.draft // false' <<<"$PR_JSON")" + +missing=() +# Deduped set of doc-link topics, one per kind of failing check. render_status_body +# maps these keys to the matching "How to fix" links so only relevant links are shown. +topics="" +add_topic() { case " $topics " in *" $1 "*) ;; *) topics="${topics}${topics:+ }$1" ;; esac; } + +m="$(check_title "$title")"; if [[ -n "$m" ]]; then missing+=("$m"); add_topic commit; fi +m="$(check_linked_issue "$body")"; if [[ -n "$m" ]]; then missing+=("$m"); add_topic issue; fi +m="$(check_ai_disclosure "$body")"; if [[ -n "$m" ]]; then missing+=("$m"); add_topic workflow; fi + +merge_shas=() +while IFS= read -r commit; do + [[ -z "$commit" ]] && continue + sha="$(jq -r '.sha[0:7]' <<<"$commit")" + parents="$(jq -r '.parents | length' <<<"$commit")" + if (( parents >= 2 )); then merge_shas+=("$sha"); continue; fi # merge commit: flag below, skip format checks + message="$(jq -r '.commit.message' <<<"$commit")" + subject="${message%%$'\n'*}" + m="$(check_commit_subject "$sha" "$subject")"; if [[ -n "$m" ]]; then missing+=("$m"); add_topic commit; fi + m="$(check_commit_coauthor "$sha" "$message")"; if [[ -n "$m" ]]; then missing+=("$m"); add_topic workflow; fi +done < <(jq -c '.[]' <<<"$COMMITS_JSON") + +# Merge commits are not allowed: ask the author to rebase onto the base branch. +if [[ ${#merge_shas[@]} -gt 0 ]]; then + merge_list="" + for s in "${merge_shas[@]}"; do merge_list="${merge_list:+$merge_list, }\`${s}\`"; done + missing+=("Merge commit(s) found (${merge_list}) โ€” rebase onto the base branch instead of merging.") + add_topic workflow +fi + +if [[ ${#missing[@]} -eq 0 ]]; then compliant=true; else compliant=false; fi + +exempt_reason="$(is_exempt "$author_type")" +if [[ -n "$exempt_reason" ]]; then exempt=true; else exempt=false; fi + +missing_markdown="" +if [[ ${#missing[@]} -gt 0 ]]; then + for item in "${missing[@]}"; do missing_markdown+="- [ ] ${item}"$'\n'; done +fi + +jq -n \ + --argjson compliant "$compliant" \ + --argjson exempt "$exempt" \ + --arg exempt_reason "$exempt_reason" \ + --argjson draft "$draft" \ + --arg missing_markdown "$missing_markdown" \ + --arg topics "$topics" \ + '{compliant:$compliant, exempt:$exempt, exempt_reason:$exempt_reason, draft:$draft, missing_markdown:$missing_markdown, topics:$topics}' diff --git a/scripts/ci/pr-sentinel/lib.sh b/scripts/ci/pr-sentinel/lib.sh new file mode 100755 index 0000000000..a71bd18dd5 --- /dev/null +++ b/scripts/ci/pr-sentinel/lib.sh @@ -0,0 +1,151 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Shared PR Sentinel helpers for comment + label + close orchestration. +# Requires: gh, jq. Env: GH_TOKEN, GH_REPO. +# These constants are consumed by scripts that source this file. +PR_SENTINEL_LABEL_NEEDS_UPDATES="pr-sentinel: needs updates" +PR_SENTINEL_LABEL_READY_FOR_REVIEW="pr-sentinel: ready for review" +PR_SENTINEL_MARKER="" +PR_SENTINEL_ESCALATED_MARKER="" + +# Dry-run: when enabled, state-changing gh calls are logged instead of executed. +# Enable via `DRY_RUN=1` env or the `--dry-run` flag parsed by the entry scripts. +DRY_RUN="${DRY_RUN:-0}" +is_dry_run() { [[ "$DRY_RUN" == "1" || "$DRY_RUN" == "true" ]]; } + +# Wrapper for every state-changing gh invocation. In dry-run it prints the +# intended command to stderr and skips it; otherwise it runs gh (stdout muted). +gh_write() { + if is_dry_run; then + echo "[dry-run] would run: gh $*" >&2 + return 0 + fi + gh "$@" >/dev/null +} + +require_label_needs_updates() { + require_label "$PR_SENTINEL_LABEL_NEEDS_UPDATES" +} + +require_label_ready_for_review() { + require_label "$PR_SENTINEL_LABEL_READY_FOR_REVIEW" +} + +require_label() { # $1 label name + local label_name=$1 + local encoded + encoded="$(jq -rn --arg s "$label_name" '$s|@uri')" + if ! gh api "repos/{owner}/{repo}/labels/${encoded}" >/dev/null 2>&1; then + echo "::error::Label '${label_name}' is missing โ€” a maintainer must create it." + return 1 + fi +} + +find_status_comment_id() { + local pr="$1" + gh api --paginate "repos/{owner}/{repo}/issues/${pr}/comments" \ + --jq ".[] | select(.body | contains(\"${PR_SENTINEL_MARKER}\")) | .id" | head -n1 +} + +get_status_comment_created_at() { + local pr="$1" + gh api --paginate "repos/{owner}/{repo}/issues/${pr}/comments" \ + --jq ".[] | select(.body | contains(\"${PR_SENTINEL_MARKER}\")) | .created_at" | head -n1 +} + +get_status_comment_body() { + local pr="$1" + gh api --paginate "repos/{owner}/{repo}/issues/${pr}/comments" \ + --jq ".[] | select(.body | contains(\"${PR_SENTINEL_MARKER}\")) | .body" | head -n1 +} + +render_status_body() { + local missing="$1" topics="${2:-}" + + # Build the "How to fix" list from only the topics whose checks failed. + local howto="" + case " $topics " in *" commit "*) + howto+="- Conventional Commit title & commit messages: [Git Commit Guide](https://github.com/thunderbird/thunderbird-android/blob/main/docs/contributing/git-commit-guide.md#-commit-message-format)"$'\n' ;; + esac + case " $topics " in *" issue "*) + howto+="- Linking an issue: [Linking a pull request to an issue](https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)"$'\n' ;; + esac + case " $topics " in *" workflow "*) + howto+="- All checks & auto-close policy: [Contribution Workflow](https://github.com/thunderbird/thunderbird-android/blob/main/docs/contributing/contribution-workflow.md#-automated-pr-checks-pr-sentinel)"$'\n' ;; + esac + + { + echo "๐Ÿค– **PR Sentinel โ€” this PR isn't ready to merge yet**" + echo "" + echo "Please address the following so a maintainer can review:" + echo "" + printf '%s' "$missing" + echo "" + if [[ -n "$howto" ]]; then + echo "" + echo "๐Ÿ“– **How to fix**" + echo "" + printf '%s' "$howto" + echo "" + fi + echo "_This check runs automatically. Update the PR to resolve it; PRs left unresolved are auto-closed 3 days after this first notice._" + echo "" + echo "$PR_SENTINEL_MARKER" + } +} + +upsert_status_comment() { + local pr="$1" body="$2" id + id="$(find_status_comment_id "$pr")" + if [[ -n "$id" ]]; then + gh_write api -X PATCH "repos/{owner}/{repo}/issues/comments/${id}" -f body="$body" + else + gh_write api -X POST "repos/{owner}/{repo}/issues/${pr}/comments" -f body="$body" + fi +} + +delete_status_comment() { + local pr="$1" id + id="$(find_status_comment_id "$pr")" + if [[ -n "$id" ]]; then + gh_write api -X DELETE "repos/{owner}/{repo}/issues/comments/${id}" + fi +} + +add_label() { # $1=PR number, $2=label name + local pr="$1" label_name="$2" + gh_write api -X POST "repos/{owner}/{repo}/issues/${pr}/labels" \ + -f "labels[]=${label_name}" +} + +remove_label() { # $1=PR number, $2=label name (tolerates 404 when not applied) + local pr="$1" label_name="$2" encoded + encoded="$(jq -rn --arg s "$label_name" '$s|@uri')" + gh_write api -X DELETE "repos/{owner}/{repo}/issues/${pr}/labels/${encoded}" || true +} + +# --- Sentinel label state transitions (the two labels are mutually exclusive) --- + +# Non-compliant PR: drop "ready for review", then require + add "needs updates". +mark_needs_updates() { # $1=PR number + local pr="$1" + remove_label "$pr" "$PR_SENTINEL_LABEL_READY_FOR_REVIEW" + require_label_needs_updates + add_label "$pr" "$PR_SENTINEL_LABEL_NEEDS_UPDATES" +} + +# Compliant PR: drop "needs updates", then require + add "ready for review". +mark_ready_for_review() { # $1=PR number + local pr="$1" + remove_label "$pr" "$PR_SENTINEL_LABEL_NEEDS_UPDATES" + require_label_ready_for_review + add_label "$pr" "$PR_SENTINEL_LABEL_READY_FOR_REVIEW" +} + +# Draft / skipped PR: drop both Sentinel labels, add none. +clear_sentinel_labels() { # $1=PR number + local pr="$1" + remove_label "$pr" "$PR_SENTINEL_LABEL_NEEDS_UPDATES" + remove_label "$pr" "$PR_SENTINEL_LABEL_READY_FOR_REVIEW" +} diff --git a/scripts/ci/pr-sentinel/report-pr.sh b/scripts/ci/pr-sentinel/report-pr.sh new file mode 100755 index 0000000000..3a1df84675 --- /dev/null +++ b/scripts/ci/pr-sentinel/report-pr.sh @@ -0,0 +1,88 @@ +#!/usr/bin/env bash +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${SCRIPT_DIR}/lib.sh" + +usage() { + cat <<'USAGE' +Usage: report-pr.sh [--dry-run] [-h|--help] + +Evaluate a pull request and report the result: manage the consolidated PR Sentinel +status comment and the 'pr-sentinel: needs updates' / 'pr-sentinel: ready for review' +labels, and exit non-zero when the PR is non-compliant and not exempt. + +Arguments: + Pull request number to evaluate (required, numeric). + +Options: + --dry-run Log state-changing actions without executing them. + -h, --help Show this help and exit. + +Environment: + GH_TOKEN, GH_REPO Passed through to gh (required in CI). + DRY_RUN=1 Alternative to --dry-run. +USAGE +} + +PR_NUMBER="" +while [[ $# -gt 0 ]]; do + case "$1" in + -h|--help) usage; exit 0 ;; + --dry-run) DRY_RUN=1 ;; + -*) echo "Unknown option: $1" >&2; usage >&2; exit 2 ;; + *) + if [[ -n "$PR_NUMBER" ]]; then echo "Unexpected argument: $1" >&2; usage >&2; exit 2; fi + PR_NUMBER="$1" + ;; + esac + shift +done +if [[ -z "$PR_NUMBER" ]]; then + echo "Error: is required." >&2 + usage >&2 + exit 2; +fi +if [[ ! "$PR_NUMBER" =~ ^[0-9]+$ ]]; then + echo "Error: must be numeric: $PR_NUMBER" >&2 + usage >&2 + exit 2 +fi + +export DRY_RUN # read by lib.sh's gh_write + +result="$("${SCRIPT_DIR}/evaluate-pr.sh" "$PR_NUMBER")" +compliant="$(jq -r '.compliant' <<<"$result")" +exempt="$(jq -r '.exempt' <<<"$result")" +exempt_reason="$(jq -r '.exempt_reason' <<<"$result")" +draft="$(jq -r '.draft' <<<"$result")" +missing_markdown="$(jq -r '.missing_markdown' <<<"$result")" +topics="$(jq -r '.topics' <<<"$result")" + +# Draft PRs are skipped entirely: clear any prior comment/labels (e.g. when a ready +# PR is converted back to draft) so the cron won't escalate, then stop. +if [[ "$draft" == "true" ]]; then + delete_status_comment "$PR_NUMBER" + clear_sentinel_labels "$PR_NUMBER" + echo "PR #${PR_NUMBER}: draft; skipping Sentinel checks." + exit 0 +fi + +if [[ "$compliant" == "true" ]]; then + delete_status_comment "$PR_NUMBER" + mark_ready_for_review "$PR_NUMBER" + echo "PR #${PR_NUMBER}: compliant; marked ready for review." + exit 0 +fi + +# Non-compliant: post/refresh the consolidated comment first (feedback survives). +upsert_status_comment "$PR_NUMBER" "$(render_status_body "$missing_markdown" "$topics")" + +if [[ "$exempt" == "true" ]]; then + echo "PR #${PR_NUMBER}: non-compliant but exempt (${exempt_reason}); not labeling." + exit 0 +fi + +# Swap to "needs updates" (fails loud if the label is missing) and fail the check. +mark_needs_updates "$PR_NUMBER" +echo "::error::PR #${PR_NUMBER} is not ready to merge โ€” see the PR Sentinel comment." +exit 1 diff --git a/scripts/ci/pr-sentinel/tests/_asserts.sh b/scripts/ci/pr-sentinel/tests/_asserts.sh new file mode 100644 index 0000000000..bfa57e495f --- /dev/null +++ b/scripts/ci/pr-sentinel/tests/_asserts.sh @@ -0,0 +1,53 @@ +#!/usr/bin/env bash +# Shared assertion helpers for the PR Sentinel test suites. +# Source this file, then use the assert_* helpers. It initializes `fail=0`; +# each suite's main() should end with: +# if [[ "$fail" -ne 0 ]]; then echo "TESTS FAILED"; exit 1; fi +# echo "ALL TESTS PASSED" + +# `fail` is read by each sourcing suite's main(), not within this file. +export fail=0 + +assert_eq() { # $1=got $2=want $3=label + if [[ "$1" == "$2" ]]; then + echo "ok - $3" + else echo "FAIL - $3: got [$1] want [$2]" + fail=1 + fi +} + +assert_empty() { # $1=value $2=label + if [[ -z "$1" ]]; then + echo "ok - $2 (pass)" + else + echo "FAIL - $2: expected pass, got [$1]" + fail=1 + fi +} + +assert_nonempty() { # $1=value $2=label + if [[ -n "$1" ]]; then + echo "ok - $2 (fail msg present)" + else + echo "FAIL - $2: expected a message" + fail=1 + fi +} + +assert_contains() { # $1=haystack $2=needle $3=label (literal match) + if grep -qF -- "$2" <<<"$1"; then + echo "ok - $3" + else + echo "FAIL - $3: [$2] not found in output" + fail=1 + fi +} + +assert_not_contains() { # $1=haystack $2=needle $3=label (literal match) + if grep -qF -- "$2" <<<"$1"; then + echo "FAIL - $3: [$2] unexpectedly found in output" + fail=1 + else + echo "ok - $3" + fi +} diff --git a/scripts/ci/pr-sentinel/tests/all_tests.sh b/scripts/ci/pr-sentinel/tests/all_tests.sh new file mode 100755 index 0000000000..aeba41ed51 --- /dev/null +++ b/scripts/ci/pr-sentinel/tests/all_tests.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# Run every *_test.sh in this directory and report an aggregate result. +# Each suite exits non-zero on failure; this runner mirrors that. +set -uo pipefail +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +overall=0 +passed=0 +failed=0 + +for t in "$DIR"/*_test.sh; do + [[ -e "$t" ]] || continue # no matches -> skip the literal glob + name="$(basename "$t")" + echo "==> ${name}" + if bash "$t"; then + passed=$((passed + 1)) + else + failed=$((failed + 1)) + overall=1 + fi + echo +done + +echo "================================" +echo "Suites: $((passed + failed)) | passed: ${passed} | failed: ${failed}" +if [[ "$overall" -ne 0 ]]; then echo "SOME TESTS FAILED"; exit 1; fi +echo "ALL SUITES PASSED" diff --git a/scripts/ci/pr-sentinel/tests/checks_test.sh b/scripts/ci/pr-sentinel/tests/checks_test.sh new file mode 100755 index 0000000000..2fdf323ac5 --- /dev/null +++ b/scripts/ci/pr-sentinel/tests/checks_test.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# Test inputs contain literal backticks (Markdown), not command substitution. +set -uo pipefail +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +source "${DIR}/tests/_asserts.sh" +source "${DIR}/checks.sh" + +test_title() { + assert_empty "$(check_title 'feat(ui): add thing')" "title valid scoped" + assert_empty "$(check_title 'fix: bug')" "title valid basic" + assert_nonempty "$(check_title 'add thing')" "title missing type" + assert_nonempty "$(check_title 'ci: add thing')" "invalid scope" +} + +test_linked_issue() { + # fn ----------- evaluate ------------------------------------------------------------ message + assert_empty "$(check_linked_issue 'Closes #12')" "linked Closes cap" + assert_empty "$(check_linked_issue 'text fixes #9')" "linked mid-sentence" + assert_nonempty "$(check_linked_issue 'prefixes #5')" "linked substring rejected" + assert_nonempty "$(check_linked_issue 'no link')" "linked missing" + assert_empty "$(check_linked_issue 'Part of #12')" "linked via Part of" + # closing keyword inside code must NOT count (template placeholder, examples) + assert_nonempty "$(check_linked_issue 'e.g. `Closes #1234` (template placeholder)')" "linked in inline code rejected" + assert_nonempty "$(check_linked_issue $'intro\n```\nCloses #7\n```\noutro')" "linked in fenced code rejected" + assert_empty "$(check_linked_issue 'See `foo` then Closes #8')" "real link alongside inline code passes" +} + +test_ai_disclosure() { + BODY_ONE=$'## AI Disclosure\n\n- [ ] no AI\n- [x] assisted\n- [ ] created\n\n## Contribution Checklist\n\n- [x] read' + BODY_NONE=$'## AI Disclosure\n\n- [ ] no AI\n- [ ] assisted\n- [ ] created\n\n## Contribution Checklist\n\n- [x] read' + BODY_TWO=$'## AI Disclosure\n\n- [x] no AI\n- [x] assisted\n- [ ] created' + assert_empty "$(check_ai_disclosure "$BODY_ONE")" "ai exactly one" + assert_nonempty "$(check_ai_disclosure "$BODY_NONE")" "ai none" + assert_nonempty "$(check_ai_disclosure "$BODY_TWO")" "ai two" +} + +test_commit_subject() { # max description length = 70 + assert_empty "$(check_commit_subject abc1234 'feat: ok')" "commit subj valid" + assert_nonempty "$(check_commit_subject abc1234 'bad subject no type')" "commit subj bad type" + assert_nonempty "$(check_commit_subject abc1234 'build: several changes')" "commit subj bad scope" + assert_nonempty "$(check_commit_subject abc1234 "feat: $(printf 'x%.0s' {1..71})")" "commit subj desc >70" + assert_empty "$(check_commit_subject abc1234 "feat: $(printf 'x%.0s' {1..70})")" "commit subj desc ==70" +} + +test_commit_coauthor() { + assert_empty "$(check_commit_coauthor abc1234 $'feat: ok\n\nbody')" "commit no coauthor" + assert_nonempty "$(check_commit_coauthor abc1234 $'fix: x\n\nCo-authored-by: A ')" "commit coauthor" + assert_nonempty "$(check_commit_coauthor abc1234 $'fix: x\n\nCo-Authored-By: A ')" "commit coauthor pascal case" + assert_nonempty "$(check_commit_coauthor abc1234 $'fix: x\n\nco-authored-by: a ')" "commit coauthor lower" +} + +test_exemptions() { # only bots are exempt now + assert_eq "$(is_exempt Bot)" "bot" "bot is exempt" + assert_eq "$(is_exempt User)" "" "regular user not exempt" +} + +test_escalation() { + assert_eq "$(decide_escalation 0)" "none" "esc none" + assert_eq "$(decide_escalation 86400)" "escalate" "esc 24h" + assert_eq "$(decide_escalation 259200)" "close" "esc 72h" +} + +test_date_helpers() { # GNU/BSD portable + assert_eq "$(iso_to_epoch '1970-01-01T00:00:00Z')" "0" "iso_to_epoch epoch 0" + assert_eq "$(iso_to_epoch '2020-01-01T00:00:00Z')" "1577836800" "iso_to_epoch 2020" + assert_eq "$(epoch_to_display 0)" "1970-01-01 00:00 UTC" "epoch_to_display 0" +} + +main() { + test_title + test_linked_issue + test_ai_disclosure + test_commit_subject + test_commit_coauthor + test_exemptions + test_escalation + test_date_helpers + + if [[ "$fail" -ne 0 ]]; then echo "TESTS FAILED"; exit 1; fi + echo "ALL TESTS PASSED" +} + +main diff --git a/scripts/ci/pr-sentinel/tests/dryrun_test.sh b/scripts/ci/pr-sentinel/tests/dryrun_test.sh new file mode 100755 index 0000000000..1ec58e5db4 --- /dev/null +++ b/scripts/ci/pr-sentinel/tests/dryrun_test.sh @@ -0,0 +1,43 @@ +#!/usr/bin/env bash +set -uo pipefail +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +source "${DIR}/tests/_asserts.sh" + +# Fake `gh` on PATH that records each invocation to a log file. +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT +cat >"${tmp}/gh" <<'EOF' +#!/usr/bin/env bash +echo "REAL_GH $*" >>"$GH_CALL_LOG" +EOF +chmod +x "${tmp}/gh" +export PATH="${tmp}:${PATH}" +export GH_CALL_LOG="${tmp}/calls.log" + +source "${DIR}/lib.sh" + +test_dry_run_skips_gh() { # gh must NOT be invoked; a notice is printed to stderr + local err + : >"$GH_CALL_LOG" + DRY_RUN=1 + err="$(gh_write api -X POST 'repos/o/r/issues/1/comments' -f body=x 2>&1 >/dev/null)" + assert_eq "$(wc -l <"$GH_CALL_LOG" | tr -d ' ')" "0" "dry-run does not invoke gh" + assert_contains "$err" "[dry-run]" "dry-run prints a notice" +} + +test_real_mode_invokes_gh() { # gh IS invoked + : >"$GH_CALL_LOG" + DRY_RUN=0 + gh_write api -X POST 'repos/o/r/issues/1/comments' -f body=x + assert_contains "$(cat "$GH_CALL_LOG")" "REAL_GH api -X POST" "real mode invokes gh" +} + +main() { + test_dry_run_skips_gh + test_real_mode_invokes_gh + + if [[ "$fail" -ne 0 ]]; then echo "TESTS FAILED"; exit 1; fi + echo "ALL TESTS PASSED" +} + +main diff --git a/scripts/ci/pr-sentinel/tests/escalate_test.sh b/scripts/ci/pr-sentinel/tests/escalate_test.sh new file mode 100755 index 0000000000..8b73e5de5d --- /dev/null +++ b/scripts/ci/pr-sentinel/tests/escalate_test.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +set -uo pipefail +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +source "${DIR}/tests/_asserts.sh" +source "${DIR}/checks.sh" + +test_escalated_marker() { # already-escalated body must be detected so we notify only once + local body + body=$'banner\n\n' + assert_contains "$body" "pr-sentinel-escalated" "escalated marker detected" +} + +test_decide_escalation() { # boundaries just under each threshold + assert_eq "$(decide_escalation 86399)" "none" "<24h -> none" + assert_eq "$(decide_escalation 259199)" "escalate" "<72h -> escalate" +} + +main() { + test_escalated_marker + test_decide_escalation + + if [[ "$fail" -ne 0 ]]; then echo "TESTS FAILED"; exit 1; fi + echo "ALL TESTS PASSED" +} + +main diff --git a/scripts/ci/pr-sentinel/tests/evaluate_test.sh b/scripts/ci/pr-sentinel/tests/evaluate_test.sh new file mode 100755 index 0000000000..9b3bd52b8d --- /dev/null +++ b/scripts/ci/pr-sentinel/tests/evaluate_test.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +set -uo pipefail +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +source "${DIR}/tests/_asserts.sh" + +run() { PR_JSON="$1" COMMITS_JSON="$2" bash "${DIR}/evaluate-pr.sh" 1; } + +test_compliant() { # good title + body + commit + local pr out + pr=$(jq -n '{title:"feat: x", body:"Closes #1\n\n## AI Disclosure\n- [x] a\n- [ ] b\n- [ ] c", draft:false, user:{type:"User"}, author_association:"NONE"}') + out="$(run "$pr" '[{"sha":"abc1234def","parents":[{}],"commit":{"message":"feat: ok"}}]')" + assert_eq "$(jq -r .compliant <<<"$out")" "true" "compliant PR" +} + +test_noncompliant() { # missing issue + bad commit; not exempt + local pr commits out + pr=$(jq -n '{title:"bad title", body:"no link\n\n## AI Disclosure\n- [ ] a\n- [ ] b\n- [ ] c", draft:false, user:{type:"User"}, author_association:"NONE"}') + commits='[{"sha":"abc1234def","parents":[{}],"commit":{"message":"nope\n\nCo-authored-by: X "}}]' + out="$(run "$pr" "$commits")" + assert_eq "$(jq -r .compliant <<<"$out")" "false" "non-compliant PR" + assert_eq "$(jq -r .exempt <<<"$out")" "false" "not exempt" + assert_contains "$(jq -r .missing_markdown <<<"$out")" "AI Disclosure" "missing list mentions AI Disclosure" + assert_contains "$(jq -r .topics <<<"$out")" "commit" "topics include commit" + assert_contains "$(jq -r .topics <<<"$out")" "issue" "topics include issue" + assert_contains "$(jq -r .topics <<<"$out")" "workflow" "topics include workflow" +} + +test_topic_filtering() { # only the linked-issue check fails -> topics is exactly "issue" + local pr out + pr=$(jq -n '{title:"feat: x", body:"no link\n\n## AI Disclosure\n- [x] a\n- [ ] b\n- [ ] c", draft:false, user:{type:"User"}, author_association:"NONE"}') + out="$(run "$pr" '[{"sha":"abc1234","parents":[{}],"commit":{"message":"feat: ok"}}]')" + assert_eq "$(jq -r .topics <<<"$out")" "issue" "only linked-issue failing -> topics=issue" +} + +test_bot_exempt() { # bot author is exempt even when non-compliant + local pr out + pr=$(jq -n '{title:"bad", body:"", draft:false, user:{type:"Bot"}, author_association:"NONE"}') + out="$(run "$pr" '[]')" + assert_eq "$(jq -r .exempt <<<"$out")" "true" "bot exempt flag" + assert_eq "$(jq -r .exempt_reason <<<"$out")" "bot" "bot exempt reason" +} + +test_merge_commit() { # merge commit present (2 parents) -> non-compliant, asks to rebase + local pr commits out + pr=$(jq -n '{title:"feat: x", body:"Closes #1\n\n## AI Disclosure\n- [x] a\n- [ ] b\n- [ ] c", draft:false, user:{type:"User"}, author_association:"NONE"}') + commits='[{"sha":"aaaaaaa","parents":[{}],"commit":{"message":"feat: ok"}},{"sha":"m111111","parents":[{},{}],"commit":{"message":"Merge branch main"}}]' + out="$(run "$pr" "$commits")" + assert_eq "$(jq -r .compliant <<<"$out")" "false" "merge commit -> non-compliant" + assert_contains "$(jq -r .missing_markdown <<<"$out")" "Merge commit" "merge commit flagged" + assert_contains "$(jq -r .missing_markdown <<<"$out")" "rebase" "asks to rebase" + assert_contains "$(jq -r .topics <<<"$out")" "workflow" "merge -> workflow topic" +} + +test_member_not_exempt() { # author_association MEMBER is no longer exempt + local pr out + pr=$(jq -n '{title:"bad title", body:"", draft:false, user:{type:"User"}, author_association:"MEMBER"}') + out="$(run "$pr" '[]')" + assert_eq "$(jq -r .exempt <<<"$out")" "false" "member is not exempt" +} + +test_draft() { # draft flag is surfaced so report-pr.sh can skip drafts + local pr out + pr=$(jq -n '{title:"bad", body:"", draft:true, user:{type:"User"}, author_association:"NONE"}') + out="$(run "$pr" '[]')" + assert_eq "$(jq -r .draft <<<"$out")" "true" "draft flag surfaced" +} + +main() { + test_compliant + test_noncompliant + test_topic_filtering + test_bot_exempt + test_merge_commit + test_member_not_exempt + test_draft + + if [[ "$fail" -ne 0 ]]; then echo "TESTS FAILED"; exit 1; fi + echo "ALL TESTS PASSED" +} + +main diff --git a/scripts/ci/pr-sentinel/tests/labels_test.sh b/scripts/ci/pr-sentinel/tests/labels_test.sh new file mode 100644 index 0000000000..7a7b12f5a2 --- /dev/null +++ b/scripts/ci/pr-sentinel/tests/labels_test.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +set -uo pipefail +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +source "${DIR}/tests/_asserts.sh" + +# Fake `gh` that records each invocation and reports every label as existing +# (exit 0), so require_label passes without a network call. +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT +cat >"${tmp}/gh" <<'EOF' +#!/usr/bin/env bash +echo "gh $*" >>"$GH_CALL_LOG" +EOF +chmod +x "${tmp}/gh" +export PATH="${tmp}:${PATH}" +export GH_CALL_LOG="${tmp}/calls.log" +export GH_REPO="o/r" + +source "${DIR}/lib.sh" + +DEL_NEEDS="-X DELETE repos/{owner}/{repo}/issues/1/labels/pr-sentinel%3A%20needs%20updates" +DEL_READY="-X DELETE repos/{owner}/{repo}/issues/1/labels/pr-sentinel%3A%20ready%20for%20review" +ADD_NEEDS="labels[]=pr-sentinel: needs updates" +ADD_READY="labels[]=pr-sentinel: ready for review" + +test_mark_ready_for_review() { # compliant -> drop needs, add ready + local calls + : >"$GH_CALL_LOG" + mark_ready_for_review 1 + calls="$(cat "$GH_CALL_LOG")" + assert_contains "$calls" "$DEL_NEEDS" "ready: removes needs-updates" + assert_contains "$calls" "$ADD_READY" "ready: adds ready-for-review" + assert_not_contains "$calls" "$ADD_NEEDS" "ready: does not add needs-updates" +} + +test_mark_needs_updates() { # non-compliant -> drop ready, add needs + local calls + : >"$GH_CALL_LOG" + mark_needs_updates 1 + calls="$(cat "$GH_CALL_LOG")" + assert_contains "$calls" "$DEL_READY" "needs: removes ready-for-review" + assert_contains "$calls" "$ADD_NEEDS" "needs: adds needs-updates" + assert_not_contains "$calls" "$ADD_READY" "needs: does not add ready-for-review" +} + +test_clear_sentinel_labels() { # draft/skip -> drop both, add none + local calls + : >"$GH_CALL_LOG" + clear_sentinel_labels 1 + calls="$(cat "$GH_CALL_LOG")" + assert_contains "$calls" "$DEL_NEEDS" "clear: removes needs-updates" + assert_contains "$calls" "$DEL_READY" "clear: removes ready-for-review" + assert_not_contains "$calls" "-X POST" "clear: adds no label" +} + +main() { + test_mark_ready_for_review + test_mark_needs_updates + test_clear_sentinel_labels + + if [[ "$fail" -ne 0 ]]; then echo "TESTS FAILED"; exit 1; fi + echo "ALL TESTS PASSED" +} + +main