diff --git a/.github/workflows/pr-contract.yml b/.github/workflows/pr-contract.yml index a870bd7..a25b4bc 100644 --- a/.github/workflows/pr-contract.yml +++ b/.github/workflows/pr-contract.yml @@ -20,42 +20,7 @@ jobs: env: PR_BODY: ${{ github.event.pull_request.body }} PR_HEAD: ${{ github.event.pull_request.head.ref }} - run: | - set -euo pipefail - - printf '%s\n' "$PR_BODY" | grep -E "(Fixes|Closes|Resolves) #[0-9]+" >/dev/null \ - || { echo "PR body must include Fixes/Closes/Resolves #"; exit 1; } - - printf '%s\n' "$PR_BODY" | grep -E "(TDD Evidence|Tests|Test Plan|No-test justification)" >/dev/null \ - || { echo "PR body must include test evidence or a no-test justification"; exit 1; } - - if printf '%s\n' "$PR_HEAD" | grep -E '^agent/issue-[0-9]+-' >/dev/null; then - echo "Agent branch naming is valid: $PR_HEAD" - else - echo "Non-agent or legacy branch name: $PR_HEAD" - fi - - - name: Require tests or explicit no-test justification - env: + PR_AUTHOR: ${{ github.event.pull_request.user.login }} BASE_REF: ${{ github.base_ref }} - PR_BODY: ${{ github.event.pull_request.body }} run: | - set -euo pipefail - git fetch origin "$BASE_REF" --depth=1 - changed="$(git diff --name-only "origin/${BASE_REF}"...HEAD)" - printf '%s\n' "$changed" - - code_changed="$(printf '%s\n' "$changed" | grep -E '\.(py|ts|tsx|js|jsx|rs)$' || true)" - if [ -z "$code_changed" ]; then - echo "No source-code changes detected." - exit 0 - fi - - test_changed="$(printf '%s\n' "$changed" | grep -E '(^tests/|/tests/|test_|_test\.|\.spec\.|\.test\.)' || true)" - if [ -n "$test_changed" ]; then - echo "Test changes detected." - exit 0 - fi - - printf '%s\n' "$PR_BODY" | grep -Ei "no-test justification|docs-only|config-only|refactor-only" >/dev/null \ - || { echo "Code PRs require test changes or explicit no-test justification."; exit 1; } + scripts/check-pr-contract.sh diff --git a/scripts/check-pr-contract.sh b/scripts/check-pr-contract.sh new file mode 100755 index 0000000..5bbbf0b --- /dev/null +++ b/scripts/check-pr-contract.sh @@ -0,0 +1,91 @@ +#!/usr/bin/env bash +set -euo pipefail + +PR_BODY="${PR_BODY:-}" +PR_HEAD="${PR_HEAD:-}" +PR_AUTHOR="${PR_AUTHOR:-}" +BASE_REF="${BASE_REF:-}" + +changed_files() { + if [[ -n "${CHANGED_FILES:-}" ]]; then + printf '%s\n' "$CHANGED_FILES" + return + fi + + if [[ -z "$BASE_REF" ]]; then + echo "BASE_REF is required when CHANGED_FILES is not provided" >&2 + return 2 + fi + + git fetch origin "$BASE_REF" --depth=1 >/dev/null + git diff --name-only "origin/${BASE_REF}"...HEAD +} + +is_dependabot_pr() { + [[ "$PR_AUTHOR" == "dependabot[bot]" || "$PR_HEAD" == dependabot/* ]] +} + +is_dependency_maintenance_file() { + case "$1" in + .github/dependabot.yml|.github/dependabot.yaml) return 0 ;; + .github/workflows/*.yml|.github/workflows/*.yaml) return 0 ;; + package.json|package-lock.json|npm-shrinkwrap.json|pnpm-lock.yaml|yarn.lock) return 0 ;; + pyproject.toml|poetry.lock|uv.lock|requirements*.txt|setup.cfg|setup.py) return 0 ;; + Cargo.toml|Cargo.lock|go.mod|go.sum) return 0 ;; + Gemfile|Gemfile.lock|composer.json|composer.lock) return 0 ;; + Dockerfile|docker-compose.yml|docker-compose.yaml) return 0 ;; + *) return 1 ;; + esac +} + +all_changes_are_dependency_maintenance() { + local path + local saw_file=0 + while IFS= read -r path; do + [[ -z "$path" ]] && continue + saw_file=1 + if ! is_dependency_maintenance_file "$path"; then + echo "Non-dependency-maintenance change: $path" + return 1 + fi + done + [[ "$saw_file" -eq 1 ]] +} + +changed="$(changed_files)" +printf '%s\n' "$changed" + +dependabot_dependency_only=0 +if is_dependabot_pr && all_changes_are_dependency_maintenance <<<"$changed"; then + dependabot_dependency_only=1 + echo "Dependabot dependency-maintenance PR detected; body issue/test prose contract is exempt." +fi + +if [[ "$dependabot_dependency_only" -ne 1 ]]; then + printf '%s\n' "$PR_BODY" | grep -E "(Fixes|Closes|Resolves) #[0-9]+" >/dev/null \ + || { echo "PR body must include Fixes/Closes/Resolves #"; exit 1; } + + printf '%s\n' "$PR_BODY" | grep -E "(TDD Evidence|Tests|Test Plan|No-test justification)" >/dev/null \ + || { echo "PR body must include test evidence or a no-test justification"; exit 1; } +fi + +if printf '%s\n' "$PR_HEAD" | grep -E '^agent/issue-[0-9]+-' >/dev/null; then + echo "Agent branch naming is valid: $PR_HEAD" +else + echo "Non-agent or legacy branch name: $PR_HEAD" +fi + +code_changed="$(printf '%s\n' "$changed" | grep -E '\.(py|ts|tsx|js|jsx|rs)$' || true)" +if [[ -z "$code_changed" ]]; then + echo "No source-code changes detected." + exit 0 +fi + +test_changed="$(printf '%s\n' "$changed" | grep -E '(^tests/|/tests/|test_|_test\.|\.spec\.|\.test\.)' || true)" +if [[ -n "$test_changed" ]]; then + echo "Test changes detected." + exit 0 +fi + +printf '%s\n' "$PR_BODY" | grep -Ei "no-test justification|docs-only|config-only|refactor-only" >/dev/null \ + || { echo "Code PRs require test changes or explicit no-test justification."; exit 1; } diff --git a/scripts/test-pr-contract.sh b/scripts/test-pr-contract.sh new file mode 100755 index 0000000..9c7e73f --- /dev/null +++ b/scripts/test-pr-contract.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +CHECK="$ROOT_DIR/scripts/check-pr-contract.sh" + +expect_pass() { + local name="$1" + shift + if "$@"; then + echo "PASS expected: $name" + else + echo "Expected pass but failed: $name" >&2 + exit 1 + fi +} + +expect_fail() { + local name="$1" + shift + if "$@"; then + echo "Expected failure but passed: $name" >&2 + exit 1 + else + echo "FAIL expected: $name" + fi +} + +expect_fail "human PR without issue link" env \ + PR_AUTHOR="octocat" \ + PR_HEAD="feature/no-issue" \ + PR_BODY="Tests: npm test" \ + CHANGED_FILES="src/extension.ts" \ + "$CHECK" + +expect_pass "dependabot workflow-only update" env \ + PR_AUTHOR="dependabot[bot]" \ + PR_HEAD="dependabot/github_actions/actions/checkout-6" \ + PR_BODY="Bumps actions/checkout from 4 to 6." \ + CHANGED_FILES=".github/workflows/pr-contract.yml" \ + "$CHECK" + +expect_fail "dependabot source change still needs tests or justification" env \ + PR_AUTHOR="dependabot[bot]" \ + PR_HEAD="dependabot/npm/typescript-6" \ + PR_BODY="Bumps typescript." \ + CHANGED_FILES="src/extension.ts" \ + "$CHECK" + +expect_pass "agent issue branch with tests" env \ + PR_AUTHOR="newtontech" \ + PR_HEAD="agent/issue-231-dependabot-contract" \ + PR_BODY=$'Fixes #231\n\nTests: scripts/test-pr-contract.sh' \ + CHANGED_FILES=$'scripts/check-pr-contract.sh\nscripts/test-pr-contract.sh' \ + "$CHECK"