diff --git a/.github/workflows/go-pr-validation.yml b/.github/workflows/go-pr-validation.yml index 95a47f28..2638415f 100644 --- a/.github/workflows/go-pr-validation.yml +++ b/.github/workflows/go-pr-validation.yml @@ -7,6 +7,16 @@ name: "Go PR Validation" on: workflow_call: + outputs: + has_breaking_changes: + description: 'Whether the PR contains breaking changes (true/false)' + value: ${{ jobs.metadata.outputs.has_breaking_changes || 'false' }} + breaking_change_approved: + description: 'Whether the exact breaking change acknowledgement is present (true/false)' + value: ${{ jobs.metadata.outputs.breaking_change_approved || 'false' }} + breaking_change_result: + description: 'Breaking change guard result (success/failure)' + value: ${{ jobs.metadata.outputs.breaking_change_result || 'failure' }} inputs: runner_type: description: 'GitHub runner type to use' diff --git a/.github/workflows/js-pr-validation.yml b/.github/workflows/js-pr-validation.yml index 881880f5..e89ccee2 100644 --- a/.github/workflows/js-pr-validation.yml +++ b/.github/workflows/js-pr-validation.yml @@ -7,6 +7,16 @@ name: "JS/TS PR Validation" on: workflow_call: + outputs: + has_breaking_changes: + description: 'Whether the PR contains breaking changes (true/false)' + value: ${{ jobs.metadata.outputs.has_breaking_changes || 'false' }} + breaking_change_approved: + description: 'Whether the exact breaking change acknowledgement is present (true/false)' + value: ${{ jobs.metadata.outputs.breaking_change_approved || 'false' }} + breaking_change_result: + description: 'Breaking change guard result (success/failure)' + value: ${{ jobs.metadata.outputs.breaking_change_result || 'failure' }} inputs: runner_type: description: 'GitHub runner type to use' diff --git a/.github/workflows/pr-validation.yml b/.github/workflows/pr-validation.yml index 8f832237..d9d5e180 100644 --- a/.github/workflows/pr-validation.yml +++ b/.github/workflows/pr-validation.yml @@ -1,13 +1,22 @@ name: "PR Validation" -# Reusable workflow for comprehensive pull request validation -# Uses a 2-tier fail-fast model: -# Tier 1 (blocking-checks): lightweight validations that block merge — title, source branch, description -# Tier 2 (advisory-checks): informational checks — size, labels, metadata, changelog (only runs if Tier 1 passes) -# Followed by a summary job and optional Slack notification +# Reusable workflow for pull_request events only. +# A mandatory fail-closed breaking-change guard runs for every PR, including drafts, and is enforced +# through Blocking Checks. Non-draft PRs also run the existing title, source branch, description, +# advisory, reporting, and notification flow. on: workflow_call: + outputs: + has_breaking_changes: + description: 'Whether the PR contains breaking changes (true/false)' + value: ${{ jobs.breaking-change-guard.outputs.has-breaking-changes == 'true' && 'true' || 'false' }} + breaking_change_approved: + description: 'Compatibility field: whether the exact PR author acknowledgement is present (true/false); this does not grant maintainer permission' + value: ${{ jobs.breaking-change-guard.outputs.approved == 'true' && 'true' || 'false' }} + breaking_change_result: + description: 'Breaking change guard result (success/failure)' + value: ${{ jobs.breaking-change-guard.outputs.result == 'success' && 'success' || 'failure' }} inputs: runner_type: description: 'GitHub runner type to use' @@ -77,11 +86,107 @@ permissions: issues: write jobs: + # ----------------- Mandatory Breaking Change Detection (always runs, including drafts) ----------------- + breaking-change-guard: + name: Breaking Change Guard + runs-on: ${{ vars.GENERAL_RUNNERS || inputs.runner_type }} + permissions: + contents: read + outputs: + has-breaking-changes: ${{ steps.normalize.outputs.has-breaking-changes }} + approved: ${{ steps.normalize.outputs.approved }} + result: ${{ steps.normalize.outputs.result }} + detection-succeeded: ${{ steps.normalize.outputs.detection-succeeded }} + + steps: + - name: Validate original event + id: event-validation + continue-on-error: true + shell: bash + env: + EVENT_NAME: ${{ github.event_name }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + PR_BASE_REF: ${{ github.event.pull_request.base.ref }} + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: | + set -euo pipefail + + if [ "$EVENT_NAME" != "pull_request" ]; then + echo "PR Validation only accepts pull_request events; received ${EVENT_NAME:-missing}." >&2 + exit 1 + fi + + if [ -z "$PR_NUMBER" ] || [ -z "$PR_HEAD_REF" ] || [ -z "$PR_HEAD_SHA" ] \ + || [ -z "$PR_BASE_REF" ] || [ -z "$PR_BASE_SHA" ]; then + echo "The pull_request event is missing required PR number, head, or base data." >&2 + exit 1 + fi + + - name: Checkout code + id: checkout + if: steps.event-validation.outcome == 'success' + continue-on-error: true + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v6 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Detect breaking changes + id: guard + if: steps.event-validation.outcome == 'success' && steps.checkout.outcome == 'success' + continue-on-error: true + uses: LerianStudio/github-actions-shared-workflows/src/validate/breaking-change-guard@v1 + with: + base-ref: ${{ github.base_ref }} + breaking-change-acknowledgement: 'Breaking change acknowledged: I understand that this PR intentionally introduces a breaking change and requires the next release to be a major version.' + acknowledgement-match-mode: exact-visible-line + + - name: Normalize breaking change result + id: normalize + if: always() + shell: bash + env: + EVENT_VALIDATION_OUTCOME: ${{ steps.event-validation.outcome }} + CHECKOUT_OUTCOME: ${{ steps.checkout.outcome }} + GUARD_OUTCOME: ${{ steps.guard.outcome }} + RAW_HAS_BREAKING_CHANGES: ${{ steps.guard.outputs.has-breaking-changes }} + RAW_APPROVED: ${{ steps.guard.outputs.approved }} + run: | + set -euo pipefail + + has_breaking_changes=false + approved=false + result=failure + detection_succeeded=false + + if [ "$EVENT_VALIDATION_OUTCOME" = "success" ] \ + && [ "$CHECKOUT_OUTCOME" = "success" ] && [ "$GUARD_OUTCOME" = "success" ] \ + && { [ "$RAW_HAS_BREAKING_CHANGES" = "true" ] || [ "$RAW_HAS_BREAKING_CHANGES" = "false" ]; } \ + && { [ "$RAW_APPROVED" = "true" ] || [ "$RAW_APPROVED" = "false" ]; }; then + has_breaking_changes="$RAW_HAS_BREAKING_CHANGES" + approved="$RAW_APPROVED" + detection_succeeded=true + + if [ "$has_breaking_changes" = "false" ] || [ "$approved" = "true" ]; then + result=success + fi + fi + + { + echo "has-breaking-changes=$has_breaking_changes" + echo "approved=$approved" + echo "result=$result" + echo "detection-succeeded=$detection_succeeded" + } >> "$GITHUB_OUTPUT" + # ----------------- Tier 1: Blocking Checks (no checkout, fail-fast) ----------------- blocking-checks: name: Blocking Checks runs-on: ${{ vars.GENERAL_RUNNERS || inputs.runner_type }} - if: github.event.pull_request.draft != true + needs: [breaking-change-guard] + if: always() outputs: source-branch-result: ${{ steps.collect.outputs.source_branch }} title-result: ${{ steps.collect.outputs.title }} @@ -91,7 +196,7 @@ jobs: # The composite auto-skips when target branch is not in target_branches_for_source_check (default: main) - name: Validate source branch id: source-branch - if: inputs.enforce_source_branches + if: github.event_name == 'pull_request' && github.event.pull_request.draft != true && inputs.enforce_source_branches continue-on-error: true uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-source-branch@v1 with: @@ -102,6 +207,7 @@ jobs: - name: Validate PR title id: title + if: github.event_name == 'pull_request' && github.event.pull_request.draft != true continue-on-error: true uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-title@v1 with: @@ -112,11 +218,36 @@ jobs: - name: Validate PR description id: description + if: github.event_name == 'pull_request' && github.event.pull_request.draft != true continue-on-error: true - uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-description@develop + uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-description@v1 + + - name: Enforce breaking change guard + if: always() + env: + DRY_RUN: ${{ inputs.dry_run && 'true' || 'false' }} + GUARD_JOB_RESULT: ${{ needs.breaking-change-guard.result }} + GUARD_RESULT: ${{ needs.breaking-change-guard.outputs.result }} + HAS_BREAKING_CHANGES: ${{ needs.breaking-change-guard.outputs.has-breaking-changes }} + BREAKING_CHANGE_ACKNOWLEDGED: ${{ needs.breaking-change-guard.outputs.approved }} + DETECTION_SUCCEEDED: ${{ needs.breaking-change-guard.outputs.detection-succeeded }} + run: | + set -euo pipefail + + if [ "$DRY_RUN" = "true" ]; then + echo "Breaking change guard dry run: job=$GUARD_JOB_RESULT result=${GUARD_RESULT:-missing} has_breaking_changes=${HAS_BREAKING_CHANGES:-missing} acknowledged=${BREAKING_CHANGE_ACKNOWLEDGED:-missing} detection_succeeded=${DETECTION_SUCCEEDED:-missing}" + exit 0 + fi + + if [ "$GUARD_JOB_RESULT" != "success" ] || [ "$GUARD_RESULT" != "success" ] \ + || [ "$DETECTION_SUCCEEDED" != "true" ]; then + echo "Breaking change guard failed: job=$GUARD_JOB_RESULT result=${GUARD_RESULT:-missing} detection_succeeded=${DETECTION_SUCCEEDED:-missing}" >&2 + exit 1 + fi - name: Collect results and enforce blocking id: collect + if: always() && github.event_name == 'pull_request' && github.event.pull_request.draft != true uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-blocking-collect@v1 with: source-branch-outcome: ${{ steps.source-branch.outcome || 'skipped' }} @@ -128,7 +259,7 @@ jobs: name: Advisory Checks runs-on: ${{ vars.GENERAL_RUNNERS || inputs.runner_type }} needs: [blocking-checks] - if: always() && needs.blocking-checks.result == 'success' && github.event.pull_request.draft != true + if: always() && github.event_name == 'pull_request' && needs.blocking-checks.result == 'success' && github.event.pull_request.draft != true outputs: metadata-result: ${{ steps.collect.outputs.metadata }} size-result: ${{ steps.collect.outputs.size }} @@ -179,7 +310,7 @@ jobs: pr-checks-summary: name: PR Checks Summary runs-on: ${{ inputs.pr_checks_summary_runner_type || vars.GENERAL_RUNNERS || inputs.runner_type }} - needs: [blocking-checks, advisory-checks] + needs: [breaking-change-guard, blocking-checks, advisory-checks] if: always() steps: @@ -192,14 +323,106 @@ jobs: size-result: ${{ needs.advisory-checks.outputs.size-result || 'skipped' }} label-result: ${{ needs.advisory-checks.outputs.label-result || 'skipped' }} metadata-result: ${{ needs.advisory-checks.outputs.metadata-result || 'skipped' }} + breaking-change-result: ${{ needs.breaking-change-guard.result == 'success' && needs.breaking-change-guard.outputs.result == 'success' && 'success' || 'failure' }} + blocking-checks-result: ${{ needs.blocking-checks.result == 'success' && 'success' || 'failure' }} dry-run: ${{ inputs.dry_run && 'true' || 'false' }} + # ----------------- Breaking Change Feedback Comment (best effort) ----------------- + breaking-change-comment: + name: Breaking Change Comment + runs-on: ${{ vars.GENERAL_RUNNERS || inputs.runner_type }} + needs: [breaking-change-guard] + if: always() && github.event_name == 'pull_request' && github.event.pull_request.draft != true && !inputs.dry_run + permissions: + pull-requests: read + issues: write + + steps: + - name: Manage breaking change guard comment + continue-on-error: true + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GUARD_JOB_RESULT: ${{ needs.breaking-change-guard.result }} + DETECTION_SUCCEEDED: ${{ needs.breaking-change-guard.outputs.detection-succeeded }} + HAS_BREAKING_CHANGES: ${{ needs.breaking-change-guard.outputs.has-breaking-changes }} + BREAKING_CHANGE_ACKNOWLEDGED: ${{ needs.breaking-change-guard.outputs.approved }} + EVENT_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + ACKNOWLEDGEMENT: 'Breaking change acknowledged: I understand that this PR intentionally introduces a breaking change and requires the next release to be a major version.' + with: + github-token: ${{ github.token }} + script: | + const marker = ''; + const guardJobSucceeded = process.env.GUARD_JOB_RESULT === 'success'; + const detectionSucceeded = process.env.DETECTION_SUCCEEDED === 'true'; + const hasBreakingChanges = process.env.HAS_BREAKING_CHANGES === 'true'; + const acknowledged = process.env.BREAKING_CHANGE_ACKNOWLEDGED === 'true'; + const eventHeadSha = process.env.EVENT_HEAD_SHA; + const acknowledgement = process.env.ACKNOWLEDGEMENT; + + const current = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.issue.number, + }); + if (!eventHeadSha || current.data.head.sha !== eventHeadSha) { + core.warning(`Skipping stale breaking change comment update: event head ${eventHeadSha || 'missing'}, current head ${current.data.head.sha}.`); + return; + } + + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + per_page: 100, + }); + const owned = comments.filter(comment => + comment.user?.login === 'github-actions[bot]' + && (comment.body === marker || comment.body?.startsWith(`${marker}\n`)) + ); + const deleteComment = comment => github.rest.issues.deleteComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: comment.id, + }); + const upsertOwned = async body => { + const params = { owner: context.repo.owner, repo: context.repo.repo, body }; + + if (owned.length > 0) { + await github.rest.issues.updateComment({ ...params, comment_id: owned[0].id }); + await Promise.all(owned.slice(1).map(deleteComment)); + return; + } + + await github.rest.issues.createComment({ + ...params, + issue_number: context.issue.number, + }); + }; + + if (!guardJobSucceeded || !detectionSucceeded) { + const body = `${marker}\n## Breaking change guard\n\n**Status:** ⚠️ Detection failed\n\nThe workflow could not determine whether this pull request contains breaking changes. Blocking Checks failed closed.`; + await upsertOwned(body); + return; + } + + if (!hasBreakingChanges) { + await Promise.all(owned.map(deleteComment)); + return; + } + + const state = acknowledged ? '✅ Author acknowledged' : '🚫 Awaiting author acknowledgement'; + const detail = acknowledged + ? 'The exact visible author acknowledgement line is present in the pull request description. It records intentional awareness and does not grant maintainer permission.' + : 'The PR author can acknowledge intentional awareness by adding this exact visible line to the pull request description:'; + const body = `${marker}\n## Breaking change guard\n\n**Status:** ${state}\n\n${detail}\n\n\`${acknowledgement}\``; + await upsertOwned(body); + # ----------------- PR Validation Summary Comment ----------------- pr-validation-report: name: PR Validation Report runs-on: ${{ vars.GENERAL_RUNNERS || inputs.runner_type }} - needs: [blocking-checks, advisory-checks] - if: always() && github.event.pull_request.draft != true + needs: [breaking-change-guard, blocking-checks, advisory-checks] + if: always() && github.event_name == 'pull_request' && github.event.pull_request.draft != true steps: - name: Post PR validation summary comment @@ -213,17 +436,19 @@ jobs: size-result: ${{ needs.advisory-checks.outputs.size-result || 'skipped' }} label-result: ${{ needs.advisory-checks.outputs.label-result || 'skipped' }} metadata-result: ${{ needs.advisory-checks.outputs.metadata-result || 'skipped' }} + breaking-change-result: ${{ needs.breaking-change-guard.result == 'success' && needs.breaking-change-guard.outputs.result == 'success' && 'success' || 'failure' }} + blocking-checks-result: ${{ needs.blocking-checks.result == 'success' && 'success' || 'failure' }} dry-run: ${{ inputs.dry_run && 'true' || 'false' }} # ----------------- Slack Notification ----------------- notify: name: Notify - needs: [blocking-checks, advisory-checks, pr-checks-summary] - if: always() && github.event.pull_request.draft != true && !inputs.dry_run + needs: [breaking-change-guard, blocking-checks, advisory-checks, pr-checks-summary] + if: always() && github.event_name == 'pull_request' && github.event.pull_request.draft != true && !inputs.dry_run uses: LerianStudio/github-actions-shared-workflows/.github/workflows/slack-notify.yml@v1.28.12 with: - status: ${{ (needs.blocking-checks.outputs.source-branch-result == 'failure' || needs.blocking-checks.outputs.title-result == 'failure' || needs.blocking-checks.outputs.description-result == 'failure') && 'failure' || 'success' }} + status: ${{ (needs.breaking-change-guard.result != 'success' || needs.breaking-change-guard.outputs.result != 'success' || needs.blocking-checks.result != 'success' || needs.blocking-checks.outputs.source-branch-result == 'failure' || needs.blocking-checks.outputs.title-result == 'failure' || needs.blocking-checks.outputs.description-result == 'failure') && 'failure' || 'success' }} workflow_name: "PR Validation" - failed_jobs: ${{ needs.blocking-checks.outputs.source-branch-result == 'failure' && 'Source Branch, ' || '' }}${{ needs.blocking-checks.outputs.title-result == 'failure' && 'PR Title, ' || '' }}${{ needs.blocking-checks.outputs.description-result == 'failure' && 'PR Description' || '' }} + failed_jobs: ${{ (needs.breaking-change-guard.result != 'success' || needs.breaking-change-guard.outputs.result != 'success') && 'Breaking Change Guard, ' || '' }}${{ needs.blocking-checks.result != 'success' && 'Blocking Checks, ' || '' }}${{ needs.blocking-checks.outputs.source-branch-result == 'failure' && 'Source Branch, ' || '' }}${{ needs.blocking-checks.outputs.title-result == 'failure' && 'PR Title, ' || '' }}${{ needs.blocking-checks.outputs.description-result == 'failure' && 'PR Description' || '' }} secrets: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} diff --git a/.github/workflows/self-pr-validation.yml b/.github/workflows/self-pr-validation.yml index 1d736277..4abc29d2 100644 --- a/.github/workflows/self-pr-validation.yml +++ b/.github/workflows/self-pr-validation.yml @@ -72,6 +72,9 @@ jobs: - name: Run breaking-change detector matrix run: bash src/validate/breaking-change-guard/test.sh + - name: Run breaking-change workflow integration tests + run: python3 src/validate/breaking-change-guard/test-workflow.py + # ----------------- YAML Lint ----------------- yamllint: name: YAML Lint diff --git a/docs/go-pr-validation.md b/docs/go-pr-validation.md index 43ac23f5..a7aa7878 100644 --- a/docs/go-pr-validation.md +++ b/docs/go-pr-validation.md @@ -8,10 +8,11 @@ Umbrella reusable workflow for Go service repositories. A caller references this single workflow and it orchestrates everything a Go service PR needs: 1. **PR metadata** — title, source branch, size, labels (delegates to `pr-validation.yml`). -2. **Change gate** — detects whether the PR touches anything beyond docs/meta (`src/config/non-doc-changes`); documentation-only PRs skip the heavy pipelines. -3. **Go analysis** — lint, tests, coverage and build (delegates to `go-pr-analysis.yml`), opt-in via `run_go_analysis`. -4. **Security scan** — Trivy, CodeQL, prerelease checks (delegates to `pr-security-scan.yml`), opt-in via `run_security`. -5. **Lerian lib version check** — fails when a direct Lerian library is behind its latest stable release (delegates to `lerian-lib-version-check.yml`), opt-in via `run_lib_version_check`. +2. **Breaking Change Guard** — mandatory detection and enforcement inherited from `pr-validation.yml` for every PR target branch. +3. **Change gate** — detects whether the PR touches anything beyond docs/meta (`src/config/non-doc-changes`); documentation-only PRs skip the heavy pipelines. +4. **Go analysis** — lint, tests, coverage and build (delegates to `go-pr-analysis.yml`), opt-in via `run_go_analysis`. +5. **Security scan** — Trivy, CodeQL, prerelease checks (delegates to `pr-security-scan.yml`), opt-in via `run_security`. +6. **Lerian lib version check** — fails when a direct Lerian library is behind its latest stable release (delegates to `lerian-lib-version-check.yml`), opt-in via `run_lib_version_check`. The `go-analysis`, `security` and `lib-version` pipelines each have a `*-gate` aggregator job that exposes a single stable status-check name (`Go Analysis`, `Security`, `Lib Version`) for branch protection, regardless of the internal job names. All three are gated by the change detector, so documentation-only PRs skip them (and the aggregators still report success). If the change detector (`changes`) job itself fails, the aggregators propagate that failure instead of passing — so broken change detection cannot let the required checks go green. @@ -72,6 +73,30 @@ The `go-analysis`, `security` and `lib-version` pipelines each have a `*-gate` a | `trivy_skip_dirs` | Comma-separated directories to skip in every Trivy filesystem scan (appended to the built-in skip list). Useful for excluding sub-modules from the root scan (e.g. `"tools/mock-sta-server"`). | string | `''` | | `shared_paths` | Path patterns that trigger analysis/security for all components | string | `''` | +The Breaking Change Guard has no input, enable flag, target-branch filter, or opt-out. This umbrella inherits the guard automatically from `pr-validation.yml`. + +## Outputs + +| Output | Values | Description | +|--------|--------|-------------| +| `has_breaking_changes` | `true` / `false` | Whether the PR contains at least one breaking-change commit | +| `breaking_change_approved` | `true` / `false` | Whether the PR description contains the exact acknowledgement | +| `breaking_change_result` | `success` / `failure` | Normalized guard result used by the nested `Blocking Checks` job | + +These outputs forward the nested `pr-validation` job outputs with fail-closed fallbacks at the umbrella boundary: an absent nested value becomes `false`, `false`, and `failure`, respectively. + +## Breaking Change Guard + +When a PR contains a breaking change, its description must contain this exact, case-sensitive line: + +```text +Breaking change acknowledged: I understand that this PR intentionally introduces a breaking change and requires the next release to be a major version. +``` + +The guard is mandatory for every PR target branch. PRs without the required acknowledgement fail in the existing `Blocking Checks` job, including drafts. `dry_run: true` reports detection and approval without enforcing the guard. + +Caller triggers must include the five activity types in the usage example. `edited` is mandatory so removing or adding the acknowledgement reruns validation. `ready_for_review` is retained for complete validation transitions even though the guard enforces drafts. + ## Secrets | Secret | Description | Required | @@ -86,7 +111,6 @@ The `go-analysis`, `security` and `lib-version` pipelines each have a `*-gate` a name: PR Validation on: pull_request: - branches: [develop, release-candidate, main] types: [opened, edited, synchronize, reopened, ready_for_review] permissions: @@ -121,7 +145,7 @@ jobs: ## Branch protection -Require the aggregator checks `Go Analysis`, `Security` and `Lib Version` (plus the PR metadata checks from `pr-validation.yml`). These names are stable even when the underlying analysis matrix changes. +Require the aggregator checks `Go Analysis`, `Security` and `Lib Version` (plus the PR metadata checks from `pr-validation.yml`). Breaking-change enforcement remains inside the existing `Blocking Checks` status; it does not add a branch-protection check. These names are stable even when the underlying analysis matrix changes. ## Related diff --git a/docs/js-pr-validation.md b/docs/js-pr-validation.md index 50581e66..7ea873bd 100644 --- a/docs/js-pr-validation.md +++ b/docs/js-pr-validation.md @@ -8,9 +8,10 @@ Umbrella reusable workflow for JavaScript/TypeScript repositories. A caller references this single workflow and it orchestrates everything a JS/TS PR needs: 1. **PR metadata** — title, source branch, size, labels (delegates to `pr-validation.yml`). -2. **Change gate** — detects whether the PR touches anything beyond docs/meta (`src/config/non-doc-changes`); documentation-only PRs skip the heavy pipelines. -3. **Frontend analysis** — lint, typecheck, npm audit, tests, coverage and build (delegates to `frontend-pr-analysis.yml`), opt-in via `run_frontend_analysis`. -4. **Security scan** — Trivy, CodeQL, prerelease checks (delegates to `pr-security-scan.yml`), opt-in via `run_security`. +2. **Breaking Change Guard** — mandatory detection and enforcement inherited from `pr-validation.yml` for every PR target branch. +3. **Change gate** — detects whether the PR touches anything beyond docs/meta (`src/config/non-doc-changes`); documentation-only PRs skip the heavy pipelines. +4. **Frontend analysis** — lint, typecheck, npm audit, tests, coverage and build (delegates to `frontend-pr-analysis.yml`), opt-in via `run_frontend_analysis`. +5. **Security scan** — Trivy, CodeQL, prerelease checks (delegates to `pr-security-scan.yml`), opt-in via `run_security`. The `frontend-analysis` and `security` pipelines each have a `*-gate` aggregator job that exposes a single stable status-check name (`Frontend Analysis`, `Security`) for branch protection, regardless of the internal job names. Both are gated by the change detector, so documentation-only PRs skip them (and the aggregators still report success). If the change detector (`changes`) job itself fails, the aggregators propagate that failure instead of passing. @@ -81,6 +82,30 @@ The `frontend-analysis` and `security` pipelines each have a `*-gate` aggregator > **Monorepo note:** `filter_paths`/`shared_paths`/`path_level`/`normalize_to_filter` scope the `frontend-analysis` job only. They are not passed to the `security` job because `frontend-pr-analysis.yml` and `pr-security-scan.yml` use different formats for that input (JSON array vs. newline-separated). For a path-scoped security scan too, call `pr-security-scan.yml` directly. +The Breaking Change Guard has no input, enable flag, target-branch filter, or opt-out. This umbrella inherits the guard automatically from `pr-validation.yml`. + +## Outputs + +| Output | Values | Description | +|--------|--------|-------------| +| `has_breaking_changes` | `true` / `false` | Whether the PR contains at least one breaking-change commit | +| `breaking_change_approved` | `true` / `false` | Whether the PR description contains the exact acknowledgement | +| `breaking_change_result` | `success` / `failure` | Normalized guard result used by the nested `Blocking Checks` job | + +These outputs forward the nested `pr-validation` job outputs with fail-closed fallbacks at the umbrella boundary: an absent nested value becomes `false`, `false`, and `failure`, respectively. + +## Breaking Change Guard + +When a PR contains a breaking change, its description must contain this exact, case-sensitive line: + +```text +Breaking change acknowledged: I understand that this PR intentionally introduces a breaking change and requires the next release to be a major version. +``` + +The guard is mandatory for every PR target branch. PRs without the required acknowledgement fail in the existing `Blocking Checks` job, including drafts. `dry_run: true` reports detection and approval without enforcing the guard. + +Caller triggers must include the five activity types in the usage example. `edited` is mandatory so removing or adding the acknowledgement reruns validation. `ready_for_review` is retained for complete validation transitions even though the guard enforces drafts. + ## Secrets | Secret | Description | Required | @@ -96,7 +121,6 @@ All other secrets required by the underlying primitives (e.g. `DOCKER_USERNAME`, name: PR Validation on: pull_request: - branches: [develop, release-candidate, main] types: [opened, edited, synchronize, reopened, ready_for_review] permissions: @@ -152,7 +176,7 @@ jobs: ## Branch protection -Require the aggregator checks `Frontend Analysis` and `Security` (plus the PR metadata checks from `pr-validation.yml`). These names are stable even when the underlying analysis steps change. +Require the aggregator checks `Frontend Analysis` and `Security` (plus the PR metadata checks from `pr-validation.yml`). Breaking-change enforcement remains inside the existing `Blocking Checks` status; it does not add a branch-protection check. These names are stable even when the underlying analysis steps change. ## Related diff --git a/docs/pr-validation.md b/docs/pr-validation.md index 9858ef8d..fb531933 100644 --- a/docs/pr-validation.md +++ b/docs/pr-validation.md @@ -5,7 +5,7 @@ -Comprehensive pull request validation workflow that enforces best practices, coding standards, and project conventions. Automatically checks PR title format, size, description quality, and ensures proper documentation. +Comprehensive pull request validation workflow that enforces best practices, coding standards, project conventions, and explicit PR author acknowledgement of breaking changes. ## Features @@ -14,23 +14,28 @@ Comprehensive pull request validation workflow that enforces best practices, cod - **Description quality** — Minimum length and required sections - **Auto-labeling** — Based on changed files - **Auto-assign** — Assigns PR author when no assignee is set (skips bots) -- **Draft PR support** — Skips validations for draft PRs +- **Draft PR support** — Runs and enforces the mandatory guard (and still writes the step summary) while deferring title, branch, description, advisory, PR comments, reporter output, guard comments, and Slack notifications until ready for review - **Source branch validation** — Enforce PRs to protected branches come from specific source branches +- **Mandatory breaking change guard** — Detects breaking-change commits on every target branch and blocks breaking changes without PR author acknowledgement - **Dry run mode** — Preview validations without posting comments or labels - **Summary report** — Aggregated validation status (step summary + idempotent PR comment) - **Idempotent feedback** — Source branch failures and the mergeability summary are upserted via stable markers (no stacked duplicates across commits) ## Architecture -Uses a **2-tier fail-fast model** to minimize runner cost and provide fast feedback: +Accepts original `pull_request` events only. It uses a mandatory fail-closed guard followed by a **2-tier fail-fast model** to minimize runner cost and provide fast feedback: ``` pr-validation.yml (reusable workflow) - Tier 1 — blocking-checks (no checkout, ~5s) - ├── src/validate/pr-source-branch (source branch check) - ├── src/validate/pr-title (semantic title check) - └── src/validate/pr-description (description quality) + breaking-change-guard (validates the event and always detects, including drafts) + └── src/validate/breaking-change-guard@v1 (exact visible-line acknowledgement) + ↓ + Tier 1 — blocking-checks (always enforces the guard; no checkout, ~5s) + ├── breaking-change enforcement (also runs for drafts) + ├── src/validate/pr-source-branch (non-draft source branch check) + ├── src/validate/pr-title (non-draft semantic title check) + └── src/validate/pr-description (non-draft description quality) ↓ (only continues if all pass) Tier 2 — advisory-checks (shared checkout) ├── src/validate/pr-metadata (assignee + linked issues) @@ -39,24 +44,27 @@ pr-validation.yml (reusable workflow) ↓ Summary — pr-checks-summary (always runs, step summary) ↓ - Report — pr-validation-reporter (always runs, PR comment) + Reports — breaking-change comment + pr-validation-reporter ↓ Notify — slack-notify.yml (optional) ``` -**Cost optimization:** 4 runners instead of 9, 1 checkout instead of 3. +Breaking-change enforcement reuses the existing `Blocking Checks` job. Callers do not need a new branch-protection check. ## Usage ### Basic Usage +The caller must use the `pull_request` event. `pull_request_target`, `workflow_dispatch`, and calls without complete pull request event data fail closed. Do not add a target-branch filter to the trigger: the mandatory guard applies to every PR target branch. + +The `edited` trigger is mandatory because PR body changes can add or remove the acknowledgement. Keep `ready_for_review` so a draft-to-ready transition runs the deferred validations. + ```yaml name: PR Validation on: pull_request: - branches: [develop, release-candidate, main] - types: [opened, synchronize, reopened, ready_for_review] + types: [opened, synchronize, reopened, edited, ready_for_review] permissions: contents: read @@ -65,7 +73,7 @@ permissions: jobs: validate: - uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-validation.yml@v1.2.3 + uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-validation.yml@v1.53.0 secrets: inherit ``` @@ -74,7 +82,7 @@ jobs: ```yaml jobs: validate: - uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-validation.yml@v1.2.3 + uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-validation.yml@v1.53.0 with: pr_title_types: | feat @@ -84,7 +92,6 @@ jobs: test chore require_scope: true - min_description_length: 100 enable_auto_labeler: true secrets: inherit ``` @@ -94,11 +101,10 @@ jobs: ```yaml jobs: validate: - uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-validation.yml@v1.2.3 + uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-validation.yml@v1.53.0 with: enforce_source_branches: true allowed_source_branches: 'develop|release-candidate|hotfix/*' - target_branches_for_source_check: 'main' secrets: inherit ``` @@ -107,7 +113,7 @@ jobs: ```yaml jobs: validate: - uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-validation.yml@v1.2.3 + uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-validation.yml@v1.53.0 with: dry_run: true secrets: inherit @@ -123,13 +129,47 @@ jobs: | `pr_title_types` | string | (see below) | Allowed commit types (newline-separated) | | `pr_title_scopes` | string | `''` | Allowed scopes (newline-separated, empty = any) | | `require_scope` | boolean | `false` | Require scope in PR title | -| `min_description_length` | number | `50` | Minimum PR description length | | `enable_auto_labeler` | boolean | `true` | Enable automatic labeling | | `labeler_config_path` | string | `.github/labeler.yml` | Path to labeler config | | `enforce_source_branches` | boolean | `true` | Enforce source branch rules (auto-skips when target is not in `target_branches_for_source_check`) | | `allowed_source_branches` | string | `develop\|release-candidate\|hotfix/*` | Allowed source branches (pipe-separated, supports `*` wildcard) | | `target_branches_for_source_check` | string | `main` | Target branches that require source branch validation | +The breaking change guard has no enable input, target-branch input, acknowledgement input, or opt-out. It applies to every caller and every PR target branch. `dry_run: true` remains a global preview mode without guard enforcement; it is not a guard-specific opt-out and does not change normal `dry_run: false` operation. Existing callers must migrate their `pull_request.types` list to include both `edited` and `ready_for_review`; otherwise body edits and draft-to-ready transitions do not rerun validation. + +## Outputs + +| Output | Values | Description | +|--------|--------|-------------| +| `has_breaking_changes` | `true` / `false` | Whether the PR contains at least one breaking-change commit | +| `breaking_change_approved` | `true` / `false` | Compatibility field name: whether the PR description contains the exact author acknowledgement. It records intentional awareness and does not grant maintainer permission. | +| `breaking_change_result` | `success` / `failure` | Normalized guard result used by `Blocking Checks`, reports, and notifications | + +Outputs are closed by default. Missing, skipped, or cancelled guard state returns `has_breaking_changes: false`, `breaking_change_approved: false`, and `breaking_change_result: failure`. The `breaking_change_approved` name is retained only as a compatibility contract. Consumers must interpret `true` as author acknowledgement, not maintainer permission. + +## Breaking Change Author Acknowledgement + +When a PR contains a breaking-change commit, its description must contain this exact, case-sensitive, standalone visible line: + +```text +Breaking change acknowledged: I understand that this PR intentionally introduces a breaking change and requires the next release to be a major version. +``` + +Partial text, case changes, similar wording, hidden content, blockquotes, and fenced code do not acknowledge the breaking change. The line must be visible as its own line in the rendered PR description. Any PR author can supply it. The line proves intentional awareness of the breaking change and expected major release; it does not represent maintainer permission. A breaking change with the exact visible line passes. A breaking change without it fails. A PR without breaking changes passes. + +The guard is mandatory on every target branch. There is no opt-out. The action stays on the released `@v1` line; the acknowledgement describes release intent but does not classify or select a release version. + +### Feedback comments + +For every non-draft live run, including detector failure or cancellation, the workflow uses `github.token` to manage one best-effort comment owned only when the author is exactly `github-actions[bot]` and the body starts with the exact marker line: + +- A breaking change creates or updates the comment with `Author acknowledged` or `Awaiting author acknowledgement` state and the exact acknowledgement. +- Detection or guard-job failure replaces stale acknowledgement with one `Detection failed` comment and deletes duplicate owned comments. +- Duplicate owned marker comments are deleted. +- When breaking changes disappear, all owned guard comments are deleted. +- Before any comment mutation, the workflow fetches the current PR and stops if its current head SHA differs from the event head SHA. +- Fork token write restrictions and comment API failures never affect enforcement. The workflow never falls back to another bot identity. + ### Default PR Title Types ``` @@ -147,16 +187,35 @@ feat fix docs style refactor perf test chore ci build revert | Job | Tier | Composites | Condition | |-----|------|------------|-----------| -| `blocking-checks` | 1 (fail-fast) | `pr-source-branch`, `pr-title`, `pr-description` | non-draft | +| `breaking-change-guard` | mandatory detection | `breaking-change-guard@v1` | always, including drafts | +| `blocking-checks` | 1 (fail-fast) | guard enforcement, `pr-source-branch`, `pr-title`, `pr-description` | always; drafts run guard enforcement only | | `advisory-checks` | 2 (informational) | `pr-metadata`, `pr-size`, `pr-labels` | non-draft, blocking-checks passed | | `pr-checks-summary` | — | `pr-checks-summary` | always (writes to step summary) | +| `breaking-change-comment` | — | `actions/github-script` | every non-draft live run, including detector failure or cancellation | | `pr-validation-report` | — | `pr-validation-reporter` | non-draft (upserts single PR comment) | | `notify` | — | `slack-notify.yml` | non-draft, `!dry_run` | ### Blocking checks (Tier 1) - Run without checkout (lightweight, ~5 seconds) -- All three run even if one fails (`continue-on-error` per step) -- Job fails if **any** blocking check fails, preventing advisory checks from running +- Always enforce both the guard job state and its normalized output, including on drafts +- Skip existing source branch, title, description, and collector steps on drafts; their non-draft behavior is unchanged +- On non-drafts, all three existing validations run even if one fails (`continue-on-error` per step) +- Job fails if the guard job, normalized guard output, or **any** existing blocking check fails, preventing advisory checks from running +- In dry-run mode, guard state is logged but does not fail `Blocking Checks` + +### Fail-closed detection + +An event-validation step rejects any original event other than `pull_request` and rejects missing PR number, head ref/SHA, or base ref/SHA data. Therefore `pull_request_target`, `workflow_dispatch`, and incomplete PR events fail closed. + +The event validation, detector checkout, and released guard action tolerate step errors only so a final `always()` normalization step can emit deterministic outputs. Event validation failure, checkout failure, action failure, cancellation, skipped execution, or missing/malformed action outputs normalize to: + +- `breaking_change_result: failure` +- `has_breaking_changes: false` +- `breaking_change_approved: false` + +The detection job can remain successful when normalization handles the fault. Live enforcement still fails through `Blocking Checks`, which requires both the detector job result and normalized output to be `success`. If the detector job itself is cancelled or cannot emit outputs, enforcement fails closed. + +Summary and reporter inputs normalize the combined detector job and output state: only two `success` values produce guard success. They also receive the independent `Blocking Checks` job result. Slack treats every `Blocking Checks` state other than `success` as failure and reports it as `Blocking Checks`, not as a guard failure. ### Advisory checks (Tier 2) - Share a single `checkout` with `fetch-depth: 0` @@ -166,6 +225,9 @@ feat fix docs style refactor perf test chore ci build revert ## Dry Run Behavior When `dry_run: true`: +- Breaking-change detection still runs and emits deterministic outputs +- The guard prints all resolved values but does not fail `Blocking Checks` +- The breaking-change comment and validation report comments are not posted - Title, description, and metadata validations still run (read-only checks) - Size is calculated and logged but **labels are not applied** - Source branch is validated but **the failure comment is not posted/updated** @@ -175,7 +237,9 @@ When `dry_run: true`: ## Draft PR Behavior -When a PR is in draft mode, all validation jobs are skipped. Checks run automatically when the PR is marked ready for review. +When a PR is in draft mode, mandatory breaking-change detection and guard enforcement still run. `Blocking Checks` is the existing required check, so a breaking change without author acknowledgement or a detector failure blocks the draft without adding a new required check. Existing source branch, title, description, and collector steps remain skipped, and the step summary is still written. Advisory checks, PR comments, reporter output, guard comments, and Slack also remain skipped until the PR is marked ready for review. + +Every caller must include `ready_for_review` so deferred validation runs on that transition. Every caller must include `edited` so adding, changing, or removing the acknowledgement in the PR body reruns enforcement. ## PR Size Labels @@ -199,11 +263,11 @@ When a PR is in draft mode, all validation jobs are skipped. Checks run automati ## Related Workflows -- [Go CI](./go-ci.md) — Continuous integration testing -- [Go Security](./go-security.md) — Security scanning -- [PR Security Scan](./pr-security-scan.md) — Security scanning for PRs +- [Go CI](./go-ci-workflow.md) — Continuous integration testing +- [Go Security](./go-security-workflow.md) — Security scanning +- [PR Security Scan](./pr-security-scan-workflow.md) — Security scanning for PRs --- -**Last Updated:** 2026-03-25 -**Version:** 3.0.0 +**Last Updated:** 2026-08-06 +**Release line:** `v1` diff --git a/src/notify/pr-validation-reporter/README.md b/src/notify/pr-validation-reporter/README.md index 8af8c436..b7584bf4 100644 --- a/src/notify/pr-validation-reporter/README.md +++ b/src/notify/pr-validation-reporter/README.md @@ -19,15 +19,19 @@ Posts a single mergeability summary comment aggregating all PR validation check | `label-result` | Result of auto-label step | No | `skipped` | | `metadata-result` | Result of PR metadata check | No | `skipped` | | `breaking-change-result` | Result of the blocking breaking change guard | No | `skipped` | +| `blocking-checks-result` | Runtime result of the blocking checks job | No | `skipped` | | `dry-run` | When `true`, skip posting the summary comment | No | `false` | When `breaking-change-result` is `skipped` or omitted, the report omits the guard row and preserves existing mergeability behavior. This optional default exists only for backward compatibility with direct action consumers. The mandatory `pr-validation` integration always supplies the guard result and offers no guard opt-out. When supplied, only `success` is mergeable. +When `blocking-checks-result` is omitted, `skipped`, or `success`, the report remains unchanged and omits the runtime row. Any other supplied value, including `failure`, `cancelled`, an empty value, or an unknown value, adds a blocking `Blocking Checks Runtime` row and blocks the reporter verdict. This optional default exists only for backward compatibility with direct action consumers. The mandatory `pr-validation` integration always supplies this internal runtime result; it is not an opt-out. + ## Outputs | Output | Description | |--------|-------------| | `has-breaking-change-guard` | Whether the breaking change guard result was reported, i.e. `breaking-change-result` was not `skipped` (`true`/`false`) | +| `has-blocking-checks-runtime-failure` | Whether `blocking-checks-result` was supplied as a non-`success`, non-`skipped` value (`true`/`false`) | ## Usage as composite step @@ -44,7 +48,7 @@ jobs: # ...other checks... - name: Breaking Change Guard id: breaking-change-guard - uses: LerianStudio/github-actions-shared-workflows/src/validate/breaking-change-guard@v1.x.x + uses: LerianStudio/github-actions-shared-workflows/src/validate/breaking-change-guard@v1 with: base-ref: ${{ github.base_ref }} breaking-change-acknowledgement: 'BREAKING CHANGE APPROVED' @@ -71,7 +75,7 @@ jobs: if: always() && github.event.pull_request.draft != true steps: - name: Post PR Validation Summary - uses: LerianStudio/github-actions-shared-workflows/src/notify/pr-validation-reporter@v1.x.x + uses: LerianStudio/github-actions-shared-workflows/src/notify/pr-validation-reporter@v1 with: github-token: ${{ secrets.MANAGE_TOKEN || github.token }} source-branch-result: ${{ needs.blocking-checks.outputs.source-branch-result }} @@ -81,6 +85,7 @@ jobs: label-result: ${{ needs.advisory-checks.outputs.label-result }} metadata-result: ${{ needs.advisory-checks.outputs.metadata-result }} breaking-change-result: ${{ needs.blocking-checks.outputs.breaking-change-result }} + blocking-checks-result: ${{ needs.blocking-checks.result }} ``` ## Required permissions diff --git a/src/notify/pr-validation-reporter/action.yml b/src/notify/pr-validation-reporter/action.yml index 260d480f..b2f9df91 100644 --- a/src/notify/pr-validation-reporter/action.yml +++ b/src/notify/pr-validation-reporter/action.yml @@ -33,6 +33,10 @@ inputs: description: Result of breaking change guard (success/failure/cancelled/skipped) required: false default: skipped + blocking-checks-result: + description: Runtime result of the blocking checks job + required: false + default: skipped dry-run: description: When true, skip posting the summary comment required: false @@ -42,6 +46,9 @@ outputs: has-breaking-change-guard: description: Whether the breaking change guard result was reported (true/false) value: ${{ steps.guard-state.outputs.has-breaking-change-guard }} + has-blocking-checks-runtime-failure: + description: Whether the blocking checks runtime result is non-success and non-skipped (true/false) + value: ${{ steps.guard-state.outputs.has-blocking-checks-runtime-failure }} runs: using: composite @@ -51,6 +58,7 @@ runs: shell: bash env: BREAKING_CHANGE_RESULT: ${{ inputs.breaking-change-result }} + BLOCKING_CHECKS_RESULT: ${{ inputs.blocking-checks-result }} run: | set -euo pipefail if [ "$BREAKING_CHANGE_RESULT" != "skipped" ]; then @@ -58,6 +66,11 @@ runs: else echo "has-breaking-change-guard=false" >> "$GITHUB_OUTPUT" fi + if [ "$BLOCKING_CHECKS_RESULT" != "success" ] && [ "$BLOCKING_CHECKS_RESULT" != "skipped" ]; then + echo "has-blocking-checks-runtime-failure=true" >> "$GITHUB_OUTPUT" + else + echo "has-blocking-checks-runtime-failure=false" >> "$GITHUB_OUTPUT" + fi - name: Post PR validation summary if: inputs.dry-run != 'true' @@ -70,6 +83,7 @@ runs: LABEL_RESULT: ${{ inputs.label-result }} METADATA_RESULT: ${{ inputs.metadata-result }} BREAKING_CHANGE_RESULT: ${{ inputs.breaking-change-result }} + BLOCKING_CHECKS_RESULT: ${{ inputs.blocking-checks-result }} with: github-token: ${{ inputs.github-token }} script: | @@ -82,6 +96,16 @@ runs: { label: 'PR Metadata', result: process.env.METADATA_RESULT, blocking: false }, ]; + if (process.env.BLOCKING_CHECKS_RESULT !== 'success' && + process.env.BLOCKING_CHECKS_RESULT !== 'skipped') { + checks.splice(3, 0, { + label: 'Blocking Checks Runtime', + result: process.env.BLOCKING_CHECKS_RESULT, + blocking: true, + strict: true, + }); + } + if (process.env.BREAKING_CHANGE_RESULT !== 'skipped') { checks.splice(3, 0, { label: 'Breaking Change Guard', diff --git a/src/validate/breaking-change-guard/README.md b/src/validate/breaking-change-guard/README.md index 04e82690..22f576d3 100644 --- a/src/validate/breaking-change-guard/README.md +++ b/src/validate/breaking-change-guard/README.md @@ -28,9 +28,22 @@ A commit is breaking when either condition is true: `BREAKING-CHANGE` are not accepted by the configured release parser. Ordinary prose that contains a reserved footer token later in a line is not breaking. -The acknowledgement is a case-sensitive, exact literal substring of the pull request -body. It can span multiple lines and can contain regular-expression or shell -metacharacters. An empty acknowledgement is never approved. +The acknowledgement supports two matching modes: + +- `contains` preserves the original direct-action behavior. The acknowledgement is a + case-sensitive, exact literal substring of the pull request body. It can span + multiple lines and can contain regular-expression or shell metacharacters. +- `exact-visible-line` requires one visible line to equal the acknowledgement exactly. + Matching is case-sensitive and rejects leading or trailing whitespace, quoted text, + and prose that only contains the acknowledgement. Exact lines inside inline or + multiline HTML comments or Markdown fenced code blocks are ignored. Fences can be + indented and must open with at least three backticks or three tildes. A closing fence + must use the same character and be at least as long as its opening fence. Opening + fences can include a language suffix. A terminal carriage return is removed before + comparison so CRLF pull request bodies remain valid. + +An empty acknowledgement is never approved. An empty or unknown matching mode fails +closed. The guard fails closed when the repository is shallow, the remote base ref or `HEAD` is invalid, or Git cannot read the commit range. An unapproved breaking change is a @@ -42,7 +55,8 @@ its caller. | Input | Description | Required | Default | |-------|-------------|----------|---------| | `base-ref` | Pull request base branch. The checkout must contain `origin/`. | Yes | — | -| `breaking-change-acknowledgement` | Exact literal substring required in the pull request description. | Yes | — | +| `breaking-change-acknowledgement` | Exact string used to approve the breaking change. | Yes | — | +| `acknowledgement-match-mode` | Internal matching mode: `contains` or `exact-visible-line`. | No | `contains` | The composite has no `dry-run` input. Detection has no side effects. The reusable workflow owns comment, label, and merge-enforcement controls. @@ -52,7 +66,7 @@ workflow owns comment, label, and merge-enforcement controls. | Output | Description | |--------|-------------| | `has-breaking-changes` | `true` when at least one pull request head commit is breaking. | -| `approved` | `true` when the complete acknowledgement occurs in the pull request body. | +| `approved` | `true` when the pull request body satisfies the selected acknowledgement matching mode. | ## Why `actions/checkout` @@ -142,16 +156,23 @@ jobs: breaking-change-acknowledgement: Breaking change approved by the release owner. ``` -Integration with `.github/workflows/pr-validation.yml` is intentionally deferred. A -later change will add PR enforcement only after `v1` contains this composite. +Mandatory `pr-validation` integration always uses `exact-visible-line`. The workflow +does not expose the matching mode as an opt-out or configuration input. Direct +composite callers retain `contains` by default and can select `exact-visible-line` +explicitly when they need the same strict acknowledgement contract. ## Local test -Run the durable detector matrix from the repository root: +Run the durable detector and mandatory workflow-integration suites from the repository +root: ```bash bash src/validate/breaking-change-guard/test.sh +python3 src/validate/breaking-change-guard/test-workflow.py ``` -The test always runs with the default `awk`. It also runs the complete matrix with -GNU `awk` when `gawk` is installed. +The detector test always runs with the default `awk`. It also runs the complete matrix +with GNU `awk` when `gawk` is installed. The workflow test executes the Bash bodies +from `.github/workflows/pr-validation.yml` directly and verifies the mandatory guard +wiring across the reusable, umbrella, reporting, summary, and self-validation +workflows. diff --git a/src/validate/breaking-change-guard/action.yml b/src/validate/breaking-change-guard/action.yml index e6cb0caf..7687a57d 100644 --- a/src/validate/breaking-change-guard/action.yml +++ b/src/validate/breaking-change-guard/action.yml @@ -1,20 +1,24 @@ name: Breaking Change Guard -description: "Detects breaking-change commit(s) in a PR and checks whether the PR description contains the required approval acknowledgement. Reports only — the caller workflow decides whether to block merge." +description: "Detects breaking-change commit(s) in a PR and checks whether the PR description matches the required approval acknowledgement. Reports only — the caller workflow decides whether to block merge." inputs: base-ref: description: Target/base branch of the PR (used to compute the diff range of PR commits) required: true breaking-change-acknowledgement: - description: Exact string that must appear in the PR description body to approve shipping the breaking change + description: Exact string used to approve shipping the breaking change required: true + acknowledgement-match-mode: + description: Internal acknowledgement matching mode (contains or exact-visible-line) + required: false + default: contains outputs: has-breaking-changes: description: "Whether the PR contains at least one breaking-change commit (true/false)" value: ${{ steps.detect.outputs.has-breaking-changes }} approved: - description: "Whether the PR description body contains the approval acknowledgement (true/false)" + description: "Whether the PR description body matches the approval acknowledgement (true/false)" value: ${{ steps.detect.outputs.approved }} runs: @@ -26,6 +30,7 @@ runs: env: BASE_REF: ${{ inputs.base-ref }} ACK: ${{ inputs.breaking-change-acknowledgement }} + ACKNOWLEDGEMENT_MATCH_MODE: ${{ inputs.acknowledgement-match-mode }} PR_BODY: ${{ github.event.pull_request.body }} run: | set -euo pipefail diff --git a/src/validate/breaking-change-guard/detect.sh b/src/validate/breaking-change-guard/detect.sh index 324bf4ad..d0de916f 100644 --- a/src/validate/breaking-change-guard/detect.sh +++ b/src/validate/breaking-change-guard/detect.sh @@ -5,8 +5,17 @@ set -euo pipefail BASE_REF=${BASE_REF:-} PR_BODY=${PR_BODY:-} ACK=${ACK:-} +ACKNOWLEDGEMENT_MATCH_MODE=${ACKNOWLEDGEMENT_MATCH_MODE:-} AWK_BIN=${AWK:-awk} +case "${ACKNOWLEDGEMENT_MATCH_MODE}" in + contains | exact-visible-line) ;; + *) + printf 'Breaking Change Guard: ACKNOWLEDGEMENT_MATCH_MODE must be one of: contains, exact-visible-line.\n' >&2 + exit 1 + ;; +esac + if [[ -z "${BASE_REF}" ]]; then printf 'Breaking Change Guard: BASE_REF is required.\n' >&2 exit 1 @@ -85,8 +94,107 @@ has_breaking_changes=$("${AWK_BIN}" ' ' "${message_file}") approved=false -if [[ -n "${ACK}" && "${PR_BODY}" == *"${ACK}"* ]]; then - approved=true +if [[ -n "${ACK}" ]]; then + case "${ACKNOWLEDGEMENT_MATCH_MODE}" in + contains) + if [[ "${PR_BODY}" == *"${ACK}"* ]]; then + approved=true + fi + ;; + exact-visible-line) + # The awk program is intentionally a literal string. + # shellcheck disable=SC2016 + approved=$(printf '%s' "${PR_BODY}" | "${AWK_BIN}" ' + function leading_fence_length(value, character, candidate, count) { + candidate = value + sub(/^[[:space:]]*/, "", candidate) + count = 0 + while (substr(candidate, count + 1, 1) == character) { + count++ + } + return count + } + + BEGIN { + acknowledgement = ENVIRON["ACK"] + in_comment = 0 + in_fence = 0 + found = 0 + } + { + line = $0 + sub(/\r$/, "", line) + + if (in_fence) { + closing_length = leading_fence_length(line, fence_character) + closing_line = line + sub(/^[[:space:]]*/, "", closing_line) + closing_suffix = substr(closing_line, closing_length + 1) + if (closing_length >= fence_length && closing_suffix ~ /^[[:space:]]*$/) { + in_fence = 0 + } + next + } + + started_in_comment = in_comment + has_comment_marker = 0 + remainder = line + + while (length(remainder) > 0) { + if (in_comment) { + close_position = index(remainder, "-->") + if (close_position == 0) { + remainder = "" + } else { + has_comment_marker = 1 + in_comment = 0 + remainder = substr(remainder, close_position + 3) + } + } else { + open_position = index(remainder, "" "Approved for major release." \ + main "${PATH}" exact-visible-line + assert_guard "multiline HTML comment rejects hidden exact acknowledgement" false false \ + $'' "Approved for major release." \ + main "${PATH}" exact-visible-line + assert_guard "same-line HTML comment closure preserves following visible lines" false true \ + $'\nApproved for major release.' \ + "Approved for major release." main "${PATH}" exact-visible-line + assert_guard "quoted acknowledgement is not an exact visible line" false false \ + '> Approved for major release.' "Approved for major release." \ + main "${PATH}" exact-visible-line + assert_guard "acknowledgement configured with leading whitespace is rejected" false false \ + " Approved for major release." " Approved for major release." \ + main "${PATH}" exact-visible-line + assert_guard "acknowledgement configured with trailing whitespace is rejected" false false \ + "Approved for major release. " "Approved for major release. " \ + main "${PATH}" exact-visible-line + assert_guard "acknowledgement configured as a blockquote is rejected" false false \ + "> Approved for major release." "> Approved for major release." \ + main "${PATH}" exact-visible-line + assert_guard "acknowledgement configured as an unspaced blockquote is rejected" false false \ + ">Approved for major release." ">Approved for major release." \ + main "${PATH}" exact-visible-line + assert_guard "prose acknowledgement is not an exact visible line" false false \ + "Context: Approved for major release." "Approved for major release." \ + main "${PATH}" exact-visible-line + assert_guard "CRLF exact acknowledgement line" false true \ + $'Context\r\nApproved for major release.\r\nDetails\r' \ + "Approved for major release." main "${PATH}" exact-visible-line + assert_guard "backtick fenced code rejects hidden exact acknowledgement" false false \ + $'Context\n```text\nApproved for major release.\n```\nDetails' \ + "Approved for major release." main "${PATH}" exact-visible-line + assert_guard "tilde fenced code rejects hidden exact acknowledgement" false false \ + $'Context\n ~~~~markdown\nApproved for major release.\n ~~~~~\nDetails' \ + "Approved for major release." main "${PATH}" exact-visible-line + assert_guard "inline backtick text does not open a fence" false true \ + $'Context uses ```inline``` text.\nApproved for major release.' \ + "Approved for major release." main "${PATH}" exact-visible-line + assert_guard "acknowledgement after a closed fence remains visible" false true \ + $'```text\nApproved for major release.\n```\nApproved for major release.' \ + "Approved for major release." main "${PATH}" exact-visible-line + assert_guard "unclosed fence rejects hidden exact acknowledgement" false false \ + $'Context\n```text\nApproved for major release.' \ + "Approved for major release." main "${PATH}" exact-visible-line + assert_guard "contains mode preserves substring matching" false true \ + "Before. Approved for major release. After." "Approved for major release." \ + main "${PATH}" contains + assert_match_mode_failure "invalid acknowledgement match mode fails closed" invalid + assert_match_mode_failure "empty acknowledgement match mode fails closed" "" } AWK_BIN='awk' diff --git a/src/validate/pr-checks-summary/README.md b/src/validate/pr-checks-summary/README.md index 3b127007..3f89931c 100644 --- a/src/validate/pr-checks-summary/README.md +++ b/src/validate/pr-checks-summary/README.md @@ -18,15 +18,19 @@ Generates a summary table of all PR validation check results in the GitHub Actio | `label-result` | Result of auto-label step | No | `skipped` | | `metadata-result` | Result of PR metadata check | No | `skipped` | | `breaking-change-result` | Result of the breaking change guard | No | `skipped` | +| `blocking-checks-result` | Runtime result of the blocking checks job | No | `skipped` | | `dry-run` | Whether this is a dry run | No | `false` | When `breaking-change-result` is `skipped` or omitted, the summary omits the guard row and preserves existing behavior. This optional default exists only for backward compatibility with direct action consumers. The mandatory `pr-validation` integration always supplies the guard result and offers no guard opt-out. +When `blocking-checks-result` is omitted, `skipped`, or `success`, the summary remains unchanged and omits the runtime row. Any other supplied value, including `failure`, `cancelled`, an empty value, or an unknown value, adds a blocking `Blocking Checks Runtime` row. This optional default exists only for backward compatibility with direct action consumers. The mandatory `pr-validation` integration always supplies this internal runtime result; it is not an opt-out. + ## Outputs | Output | Description | |--------|-------------| | `has-breaking-change-guard` | Whether the breaking change guard result was reported, i.e. `breaking-change-result` was not `skipped` (`true`/`false`) | +| `has-blocking-checks-runtime-failure` | Whether `blocking-checks-result` was supplied as a non-`success`, non-`skipped` value (`true`/`false`) | ## Usage as composite step @@ -38,7 +42,7 @@ jobs: if: always() steps: - name: PR Checks Summary - uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-checks-summary@v1.x.x + uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-checks-summary@v1 with: source-branch-result: ${{ needs.blocking-checks.outputs.source-branch-result || 'skipped' }} title-result: ${{ needs.blocking-checks.outputs.title-result || 'skipped' }} @@ -47,6 +51,7 @@ jobs: label-result: ${{ needs.advisory-checks.outputs.label-result || 'skipped' }} metadata-result: ${{ needs.advisory-checks.outputs.metadata-result || 'skipped' }} breaking-change-result: ${{ needs.blocking-checks.outputs.breaking-change-result }} + blocking-checks-result: ${{ needs.blocking-checks.result }} dry-run: "true" ``` diff --git a/src/validate/pr-checks-summary/action.yml b/src/validate/pr-checks-summary/action.yml index 8bdeafdc..48483106 100644 --- a/src/validate/pr-checks-summary/action.yml +++ b/src/validate/pr-checks-summary/action.yml @@ -30,6 +30,10 @@ inputs: description: Result of breaking change guard required: false default: skipped + blocking-checks-result: + description: Runtime result of the blocking checks job + required: false + default: skipped dry-run: description: Whether this is a dry run required: false @@ -39,6 +43,9 @@ outputs: has-breaking-change-guard: description: Whether the breaking change guard result was reported (true/false) value: ${{ steps.guard-state.outputs.has-breaking-change-guard }} + has-blocking-checks-runtime-failure: + description: Whether the blocking checks runtime result is non-success and non-skipped (true/false) + value: ${{ steps.guard-state.outputs.has-blocking-checks-runtime-failure }} runs: using: composite @@ -48,6 +55,7 @@ runs: shell: bash env: BREAKING_CHANGE_RESULT: ${{ inputs.breaking-change-result }} + BLOCKING_CHECKS_RESULT: ${{ inputs.blocking-checks-result }} run: | set -euo pipefail if [ "$BREAKING_CHANGE_RESULT" != "skipped" ]; then @@ -55,6 +63,11 @@ runs: else echo "has-breaking-change-guard=false" >> "$GITHUB_OUTPUT" fi + if [ "$BLOCKING_CHECKS_RESULT" != "success" ] && [ "$BLOCKING_CHECKS_RESULT" != "skipped" ]; then + echo "has-blocking-checks-runtime-failure=true" >> "$GITHUB_OUTPUT" + else + echo "has-blocking-checks-runtime-failure=false" >> "$GITHUB_OUTPUT" + fi - name: Summary shell: bash @@ -66,6 +79,7 @@ runs: LABEL_RESULT: ${{ inputs.label-result }} METADATA_RESULT: ${{ inputs.metadata-result }} BREAKING_CHANGE_RESULT: ${{ inputs.breaking-change-result }} + BLOCKING_CHECKS_RESULT: ${{ inputs.blocking-checks-result }} DRY_RUN: ${{ inputs.dry-run }} run: | icon() { @@ -94,6 +108,9 @@ runs: if [ "$BREAKING_CHANGE_RESULT" != "skipped" ]; then echo "| Breaking Change Guard | $(icon "$BREAKING_CHANGE_RESULT") ${BREAKING_CHANGE_RESULT} |" fi + if [ "$BLOCKING_CHECKS_RESULT" != "success" ] && [ "$BLOCKING_CHECKS_RESULT" != "skipped" ]; then + echo "| Blocking Checks Runtime | $(icon "$BLOCKING_CHECKS_RESULT") ${BLOCKING_CHECKS_RESULT} |" + fi echo "" echo "### Advisory" echo ""