diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..4a2540d --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,33 @@ +## What is this about? + + +## Related Jira task/s + + +## Release (mandatory for every PR — required for the `ready-for-review` label) + + +**Version bump:** *(required — tick exactly one)* + +- [ ] minor (backwards-compatible feature) +- [ ] patch (bug fix or other small change) + +**Release notes type:** *(optional)* +- [ ] New Feature +- [ ] Bug Fix +- [ ] Other Improvement + +**Release notes (customer-facing):** *(optional but encouraged)* + +- + +**Release notes (internal):** *(required — engineer-facing; what actually changed / why)* + +- + +## Checklist +- [ ] Ready to review +- [ ] Has it been tested locally? + +## PR Validations +Run Tests: Comment RUN_TESTS to trigger sanity tests. diff --git a/.github/workflows/ready-for-review-label.yml b/.github/workflows/ready-for-review-label.yml new file mode 100644 index 0000000..7166b09 --- /dev/null +++ b/.github/workflows/ready-for-review-label.yml @@ -0,0 +1,220 @@ +name: Ready for Review Label + +on: + pull_request: + types: [edited, synchronize, labeled, unlabeled] + +# Coalesce runs per PR. Label add/remove re-fires this workflow; cancel-in-progress +# drops the superseded run so only the latest state is evaluated. +concurrency: + group: ready-for-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + check-ready: + # NO job-level `if`. Because Req 3 makes the JOB FAILURE the gate, this job MUST + # always conclude red/green and never "skipped" — a skipped required check reads + # as passing on GitHub, so an `if` that skips on an unrelated label event would + # silently flip the hard gate from red to green. The gate runs on every triggering + # event and does its own label filtering internally (it only ever removes the + # ready-for-review label and computes purely from the PR's current labels). + runs-on: ubuntu-latest + permissions: + pull-requests: write + issues: write + steps: + - name: Validate ready-for-review label against the release gate (SDK-6104 #8) + id: gate + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + with: + script: | + const body = context.payload.pull_request.body || ''; + // "Ready to review" is matched against the WHOLE body: the string is + // unambiguous and lives in the Checklist section, not Release. + const isCheckboxTicked = /- \[[xX]\] Ready to review/.test(body); + const labels = context.payload.pull_request.labels.map(l => l.name); + const hadLabel = labels.includes('ready-for-review'); + + // Anchor the bump count to the "## Release" section only. A bump tick + // appearing elsewhere (description prose, another checklist, a quoted + // template) must NOT satisfy the gate. Slice the body into heading blocks + // at each "## " boundary and pick the Release block. + const releaseBlock = body.split(/(?=^## )/m).find(s => /^##[ \t]*Release[ \t]*(\(|$)/m.test(s)) || ''; + const hasReleaseSection = releaseBlock !== ''; + // The free-form "Release notes (...)" subsections live INSIDE the Release + // block (bold headers, not "## " headings) and may legitimately contain + // checkbox-looking bullets (e.g. an internal note "- [x] patch the retry + // path"). Count bump ticks only in the region BEFORE the first notes + // header so notes text can never flip the count (mirrors the release + // planner's scan scoping). + const bumpRegion = releaseBlock.split(/^\*\*Release notes \(/m)[0]; + // The (?=\s|\(|$) lookahead matches a bump tick followed by whitespace, an + // opening paren ("minor (...)"), or end-of-string. It rejects e.g. "minor-stub". + const bumpTicks = [...bumpRegion.matchAll(/- \[[xX]\]\s*(minor|patch)(?=\s|\(|$)/g)].length; + + // Internal release notes are REQUIRED (they feed the internal CHANGELOG.md): + // the Release block must carry at least one non-empty bullet under + // "**Release notes (internal):**". The header-line remainder (the italic + // "*(required ...)*" annotation) and HTML comments are stripped first so + // template scaffolding can never satisfy the check, and the empty + // placeholder "- " does not count. Auto-generated release PRs (head branch + // "release_x.y.z") are EXEMPT: the release workflow writes the CHANGELOG.md + // entries itself from the constituent PRs' notes, so the release PR's own + // subsection legitimately stays empty. + const isReleasePr = /^release_\d+(\.\d+)*$/.test(context.payload.pull_request.head.ref); + const internalSplit = releaseBlock.split(/^\*\*Release notes \(internal\):\*\*/m); + const internalRegion = internalSplit.length > 1 + ? internalSplit[1].replace(/^[^\n]*\n?/, '').split(/^\*\*/m)[0].replace(//g, '') + : ''; + const internalFilled = /^[ \t]*[-*][ \t]*\S/m.test(internalRegion); + const internalOk = isReleasePr || internalFilled; + + // reasons -> full sentences for the PR comment. + // reasonBits -> short single-line fragments for step outputs / Slack. + const reasons = []; + const reasonBits = []; + if (!hasReleaseSection) { + const templateUrl = `${context.payload.repository.html_url}/blob/${context.payload.repository.default_branch}/.github/PULL_REQUEST_TEMPLATE.md`; + reasons.push(`The PR body has no \`## Release\` section. Apply the [repo PR template](${templateUrl}) to the PR body, then tick exactly one version bump.`); + reasonBits.push('missing ## Release section'); + } else if (bumpTicks === 0) { + reasons.push('Tick exactly one version bump (minor or patch) in the Release section.'); + reasonBits.push('no version bump ticked in ## Release'); + } else if (bumpTicks > 1) { + reasons.push('Tick exactly ONE version bump (minor or patch), not multiple.'); + reasonBits.push(`expected exactly one version bump tick in ## Release, found ${bumpTicks}`); + } else if (!internalOk) { + reasons.push('Fill in at least one non-empty bullet under `**Release notes (internal):**` in the Release section (engineer-facing — it feeds the internal CHANGELOG.md). The empty placeholder `- ` does not count.'); + reasonBits.push('internal release notes empty'); + } + if (!isCheckboxTicked) { + reasons.push('Tick `- [x] Ready to review` once the PR body is complete.'); + reasonBits.push('Ready-to-review checkbox not ticked'); + } + + const bumpOk = bumpTicks === 1; + const shouldHaveLabel = isCheckboxTicked && bumpOk && internalOk; + + // This workflow NEVER ADDS the ready-for-review label — the author applies + // it manually. It only REMOVES the label when the label is present but the + // body does not pass the release gate. The "Require label" step below then + // FAILS the check whenever the label is absent, making ready-for-review a + // required, manually-applied merge gate. + let removedNow = false; + if (hadLabel && !shouldHaveLabel) { + await github.rest.issues.removeLabel({ + ...context.repo, + issue_number: context.issue.number, + name: 'ready-for-review' + }).catch(() => {}); + removedNow = true; + } + // We never add, so the label is present at the end iff it was present and + // we did not strip it. + const labelPresent = hadLabel && !removedNow; + + // Warning comment (single, marker-deduped): posted when the author has + // CLAIMED readiness (ticked the checkbox) but the body fails, or when this + // run removed the label. A WIP PR that never claimed readiness gets NO + // comment — the failing check is the signal; we don't spam every draft. + // Deleted once the body passes AND the label is present. + // Emit step outputs FIRST — the notify and enforce steps depend on them, + // and they must be set regardless of the (advisory, best-effort) warning + // comment lifecycle below. + core.setOutput('removed', removedNow ? 'true' : 'false'); + core.setOutput('reason', reasonBits.join('; ') || 'PR body passes the release gate'); + core.setOutput('author', context.payload.pull_request.user.login); + core.setOutput('label_present', labelPresent ? 'true' : 'false'); + + // Warning comment lifecycle — ADVISORY ONLY, wrapped in try/catch so a transient + // GitHub comments-API error can neither (a) fail the check on a passing PR nor + // (b) skip the Slack notify step (the gate step must stay green so the later + // step `if`s evaluate). Posted when the author CLAIMED readiness (ticked the + // checkbox) but the body fails, or when this run removed the label. A WIP PR that + // never claimed readiness gets NO comment. Deleted once the body passes AND the + // label is present. + const marker = ''; + const shouldWarn = removedNow || (isCheckboxTicked && !(bumpOk && internalOk)); + let warningBody = ''; + if (shouldWarn) { + const headline = removedNow + ? '⚠️ The `ready-for-review` label was removed because the PR body does not pass the release gate:' + : '⚠️ The PR body claims it is ready to review but does not pass the release gate:'; + warningBody = marker + '\n' + headline + '\n- ' + reasons.join('\n- ') + + '\n\nHow to get (and keep) the `ready-for-review` label:\n' + + '1. Make sure the PR body follows the repo PR template (all sections present).\n' + + '2. In the `## Release` section, tick exactly one version bump (`minor` or `patch`).\n' + + '3. Fill in at least one bullet under `**Release notes (internal):**` (engineer-facing; feeds the internal CHANGELOG.md).\n' + + '4. Tick `- [x] Ready to review` in the checklist.\n' + + '5. Apply the `ready-for-review` label yourself — this workflow does NOT add it for you; it only removes it if the body stops passing, and the check stays red until the label is present.'; + } + try { + const comments = await github.paginate(github.rest.issues.listComments, { + ...context.repo, + issue_number: context.issue.number, + per_page: 100 + }); + const existingWarning = comments.find(c => (c.body || '').includes(marker)); + if (shouldWarn && !existingWarning) { + await github.rest.issues.createComment({ + ...context.repo, + issue_number: context.issue.number, + body: warningBody + }); + } else if (shouldWarn && existingWarning && existingWarning.body !== warningBody) { + await github.rest.issues.updateComment({ + ...context.repo, + comment_id: existingWarning.id, + body: warningBody + }); + } else if (labelPresent && existingWarning) { + await github.rest.issues.deleteComment({ + ...context.repo, + comment_id: existingWarning.id + }); + } + } catch (e) { + console.log(`warning-comment lifecycle skipped (non-fatal): ${e.message}`); + } + + - name: Notify Slack on label removal + if: steps.gate.outputs.removed == 'true' + env: + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_PR_SLA_WEBHOOK }} + PR_URL: ${{ github.event.pull_request.html_url }} + PR_REF: "${{ github.repository }}#${{ github.event.pull_request.number }}" + REASON: ${{ steps.gate.outputs.reason }} + AUTHOR: ${{ steps.gate.outputs.author }} + run: | + if [ -z "$SLACK_WEBHOOK_URL" ]; then + echo "SLACK_PR_SLA_WEBHOOK not configured — skipping Slack notification" + exit 0 + fi + # The author identity goes in the message TEXT and github_username is OMITTED + # from the payload. The SLACK_PR_SLA_WEBHOOK Workflow-Builder workflow resolves + # github_username -> @mention via a List lookup that ERRORS (dropping the whole + # run, so nothing posts) when the login is not in the list — e.g. an external + # PR author. Omitting github_username skips that lookup so the message ALWAYS + # surfaces (without an @mention). Plain text + bare URL (no mrkdwn link syntax). + text=":warning: ready-for-review was removed from ${PR_REF} (author: ${AUTHOR}) — ${REASON}. ${PR_URL} — fix the PR body per the repo PR template, then re-apply the label." + payload=$(jq -n --arg text "$text" '{text: $text}') + curl -fsS --max-time 10 -X POST -H 'Content-Type: application/json' \ + -d "$payload" "$SLACK_WEBHOOK_URL" \ + || echo "Slack notify failed (non-fatal)" + + - name: Require the ready-for-review label + # Hard gate: this step fails the check whenever the label is absent (a WIP PR, + # a never-applied label, or one this run just removed). It runs after the notify + # step (the gate step does not fail, so the notify is never skipped). Green only + # when the label is present — i.e. the author applied it AND the body passes + # (otherwise the gate step above would have removed it). + if: steps.gate.outputs.label_present != 'true' + run: | + echo "::error::The 'ready-for-review' label is required and is not present on this PR." + echo "Apply the 'ready-for-review' label once the PR body passes the release gate:" + echo " - the PR template is complete," + echo " - the '## Release' section has exactly one version bump (minor or patch)," + echo " - '**Release notes (internal):**' has at least one non-empty bullet," + echo " - and '- [x] Ready to review' is ticked." + echo "This workflow does not add the label for you; the check stays red until the label is present." + exit 1 diff --git a/.github/workflows/sdk-pr-review-gate.yml b/.github/workflows/sdk-pr-review-gate.yml new file mode 100644 index 0000000..adb7570 --- /dev/null +++ b/.github/workflows/sdk-pr-review-gate.yml @@ -0,0 +1,174 @@ +name: SDK PR Review Gate + +# Required check for the SDK PR Review Agent's GTG signal. Green only when BOTH: +# 1. the `ready-for-review` label is present, and +# 2. the SDK PR Review Agent's latest verdict marker reports `state=success` on the +# PR's current head SHA. The agent posts a single signed verdict comment carrying +# ``; this +# workflow reads that marker (there is no separate `sdk-pr-review` commit status, so +# the agent's outcome shows as this ONE check, never a raw status + derived check pair). +# Native PR-review approval is enforced separately (branch-protection "Require +# approvals"), not by this check. +# +# Deploy: add to each SDK repo's .github/workflows/, then require the `gate` check +# (and "Require approvals") in branch protection. + +on: + pull_request: + types: [synchronize, labeled, unlabeled] + # Re-evaluate when the SDK PR Review Agent posts/updates its verdict comment. + # (Fires from the default-branch copy once merged; pre-merge, PR events cover it.) + issue_comment: + types: [created, edited] + +# One run per PR; drop superseded runs so only the latest state is evaluated. +concurrency: + group: sdk-pr-review-gate-${{ github.event.pull_request.number || github.event.issue.number || github.event.sha }} + cancel-in-progress: true + +jobs: + gate: + # NO job-level `if`. This job IS the required check, so it must always conclude + # red/green and never "skipped" — a skipped required check reads as passing on + # GitHub. Non-applicable events (a comment on a plain issue, the gate's own + # comment edit) resolve to a green no-op, never a false pass on a real PR. + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write # required to comment on a PR (the gate's status comment) + issues: write + steps: + - name: Resolve PR + evaluate gate conditions + id: gate + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + + // issue_comment: re-evaluate ONLY when the SDK PR Review Agent adds/updates its + // verdict comment (body carries the `sdk-pr-review:verdict` marker). Everything else + // — reviewer discussion, the repo's release-gate comments, and the gate's OWN comment + // (no verdict marker) — is ignored, which also prevents self-retrigger loops. + if (context.eventName === 'issue_comment') { + if (!context.payload.issue?.pull_request) { + core.info('issue_comment on a non-PR issue — nothing to gate.'); + core.setOutput('applicable', 'false'); + return; + } + if (!(context.payload.comment?.body || '').includes('sdk-pr-review:verdict')) { + core.info('issue_comment without the verdict marker — not the agent verdict; skipping.'); + core.setOutput('applicable', 'false'); + return; + } + } + + // Resolve the PR number + head SHA for every trigger type. + let prNumber = context.payload.pull_request?.number ?? context.payload.issue?.number; + let headSha = context.payload.pull_request?.head?.sha; + + if (!prNumber) { + core.setFailed('Could not resolve a PR for this event — failing closed.'); + return; + } + core.setOutput('applicable', 'true'); + core.setOutput('pr_number', String(prNumber)); + + const pr = await github.rest.pulls.get({ owner, repo, pull_number: prNumber }); + headSha = headSha || pr.data.head.sha; + core.setOutput('author', pr.data.user.login); + + // Condition 1: ready-for-review label present. + const hasLabel = pr.data.labels.map(l => l.name).includes('ready-for-review'); + + // Condition 2: the SDK PR Review Agent's verdict marker reports `success` on the + // CURRENT head SHA only — a stale marker from a prior commit must not count. + const comments = await github.paginate(github.rest.issues.listComments, { + owner, repo, issue_number: prNumber, per_page: 100 + }); + const re = /sdk-pr-review:verdict\s+sha=(\S+)\s+state=(\w+)/g; + let verdictState = null; + for (const c of comments) { + const body = c.body || ''; + let m; + re.lastIndex = 0; + while ((m = re.exec(body)) !== null) { + if (m[1] === headSha) verdictState = m[2]; // latest matching marker wins + } + } + const reviewOk = verdictState === 'success'; + + const green = hasLabel && reviewOk; + + const reasons = []; + if (!hasLabel) reasons.push('the `ready-for-review` label is not present.'); + if (!reviewOk) { + if (verdictState === 'failure') { + reasons.push('the SDK PR Review Agent reported blocking findings (🔴) on the current head commit — fix them and re-run the agent.'); + } else if (verdictState === 'pending') { + reasons.push('the SDK PR Review Agent flagged findings that need human review (⚠️) on the current head commit — a reviewer must resolve them before this can go green.'); + } else { + reasons.push('the SDK PR Review Agent has not reviewed the current head commit yet — run the SDK PR Review Agent.'); + } + } + + core.setOutput('green', green ? 'true' : 'false'); + core.setOutput('reason', reasons.join(' ') || 'all gate conditions satisfied'); + + // Gate comment (RECURRING, but ONLY once `ready-for-review` is applied): no gate + // chatter on pre-label commits. The check still evaluates red without the label — + // this guard governs only the comment, not the gate result. When the label IS + // present, each run posts a fresh comment so the latest status sits at the bottom + // by the merge box. Best-effort — never affects the gate result. + try { + if (hasLabel) { + const marker = ''; + let body; + if (green) { + body = marker + '\n' + + '🟢 **SDK PR Review gate is green** — the SDK PR Review Agent has given a GTG for this PR ' + + '(the `ready-for-review` label is present and the latest SDK PR Review Agent run ' + + 'reports `success` on the current head commit).\n\n' + + 'A native GitHub reviewer approval is still separately required by branch protection ' + + 'before this PR can merge — this check does not substitute for that.'; + } else { + const pending = reasons.map(r => '- ' + r.charAt(0).toUpperCase() + r.slice(1)).join('\n'); + body = marker + '\n' + + '🔴 **SDK PR Review gate is red.** Pending:\n\n' + + pending + '\n\n' + + 'It turns green once the latest SDK PR Review Agent run reports GTG on the current ' + + 'head commit. A native reviewer approval is separately required by branch protection before merge.'; + } + await github.rest.issues.createComment({ owner, repo, issue_number: prNumber, body }); + } + } catch (e) { + console.log(`gate-comment (non-fatal): ${e.message}`); + } + + - name: Notify Slack on gate failure + if: steps.gate.outputs.applicable == 'true' && steps.gate.outputs.green != 'true' + env: + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_PR_SLA_WEBHOOK }} + PR_URL: "${{ github.server_url }}/${{ github.repository }}/pull/${{ steps.gate.outputs.pr_number }}" + PR_REF: "${{ github.repository }}#${{ steps.gate.outputs.pr_number }}" + REASON: ${{ steps.gate.outputs.reason }} + AUTHOR: ${{ steps.gate.outputs.author }} + run: | + if [ -z "$SLACK_WEBHOOK_URL" ]; then + echo "SLACK_PR_SLA_WEBHOOK not configured — skipping Slack notification" + exit 0 + fi + text=":no_entry: sdk-pr-review-gate is red on ${PR_REF} (author: ${AUTHOR}) — ${REASON} ${PR_URL}" + payload=$(jq -n --arg text "$text" '{text: $text}') + curl -fsS --max-time 10 -X POST -H 'Content-Type: application/json' \ + -d "$payload" "$SLACK_WEBHOOK_URL" \ + || echo "Slack notify failed (non-fatal)" + + - name: Require both gate conditions + # Hard gate: fails the required check whenever either condition (label, SDK + # PR Review Agent GTG verdict) is unmet on a real PR. No job-level `if` above + # can skip it — a skipped required check reads as passing on GitHub. + if: steps.gate.outputs.applicable == 'true' && steps.gate.outputs.green != 'true' + run: | + echo "::error::sdk-pr-review-gate is red: ${{ steps.gate.outputs.reason }}" + exit 1