From 31b42b1851ba9e5bde2a1c191a646ddb555194d7 Mon Sep 17 00:00:00 2001 From: Donavan Fritz Date: Sun, 18 Oct 2026 15:43:30 +0000 Subject: [PATCH] fix: require authorship only in PR descriptions Align the shared naming checker with the Human-directed PR-only attribution policy. Preserve visible PR provenance, naming, tracking, current identity exemptions, available base/head commits and nonempty-range validation without inspecting commit messages. Verify Human and named-Agent callers against legacy, plain, malformed, misplaced and nonfinal commit trailers; retain missing/malformed PR attribution and invalid-range rejection. All 79 offline contract tests, shell syntax and whitespace checks pass. Ticket: https://agenthub.fritzlab.net/t-d12vqnp63v5d Policy decision: https://code.fritzlab.net/websites/docs.fritzlab.net/pulls/454#issuecomment-41120 --- README.md | 16 ++++++++------- action.yaml | 2 +- check.sh | 22 ++++---------------- tests/run | 58 +++++++++++++++++++++++++++++++++++++++++++++++------ 4 files changed, 66 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index f7c35d0..890f5f1 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ # action/check-naming Composite Gitea Action that validates a pull request's branch, title, Bug -tracking, and AI authorship against the fritzlab Git standard. The current -action fails (exits 1) on any violation; `@v1` is the legacy warn-only release +tracking, PR-description AI authorship, and commit range against the fritzlab +Git standard. The current action fails (exits 1) on any violation; `@v1` is the legacy warn-only release (Bugs program: see [fritzlab/agenthub#557](https://code.fritzlab.net/fritzlab/agenthub/issues/557)). @@ -91,10 +91,12 @@ The check validates four things for every non-break-glass Agent PR: `

` sections outside collapsed `
` content. Fenced, commented, scripted, or collapsed copies do not satisfy the visible provenance contract. -4. **Commit attribution** — every commit in `base-sha..head-sha`, including - commits on `chore/` branches, ends with - the canonical `Authored-By` trailer, separated from the message body by a - blank line. The naming job must check out full history before this action. +4. **Commit range** — `base-sha` and `head-sha` must identify available commits, + and `base-sha..head-sha` must contain at least one PR commit. + The naming job must check out full history before this action. + Attribution is required only in the PR description; commit messages are not + inspected. Existing commit trailers remain valid history, but no trailer is + required for a new commit. Every violation prints a `FAIL[check-naming]: ...` line and the step exits 1. To make the check required on a repo: drop any @@ -103,7 +105,7 @@ context to the repo's `status_check_contexts` in agenthub `hub/hub.yaml`. ## Versions -- unreleased — adds Bug tracking plus PR and commit attribution enforcement. +- unreleased — requires Bug tracking and visible PR attribution, without commit-message watermarks. - `v2` — enforces branch and title naming. - `v1` — legacy warn-only: logs `WARN` lines, always exits 0. diff --git a/action.yaml b/action.yaml index 8bdf748..4b8cd91 100644 --- a/action.yaml +++ b/action.yaml @@ -1,6 +1,6 @@ name: Check PR contract description: | - Validates that a PR's branch, title, Bug tracking, and AI attribution + Validates that a PR's branch, title, Bug tracking, and PR-description AI attribution follow the fritzlab Git standard. Fails (exits 1) on any violation, with FAIL log lines naming it. Warn-only wiring is the consumer's choice: set continue-on-error: true on the job (or pin @v1, which never fails). diff --git a/check.sh b/check.sh index 0297804..7dc85f9 100755 --- a/check.sh +++ b/check.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Branch, PR body, and commit contract checker. +# Branch, PR body, and commit-range contract checker. # v2: exits 1 on violation. Consumers that want warn-only set # continue-on-error: true on the job (or stay on @v1). set -euo pipefail @@ -180,23 +180,9 @@ if [ "${BRANCH_KIND}" != "invalid" ]; then fi done if git cat-file -e "${BASE}^{commit}" 2>/dev/null && git cat-file -e "${HEAD}^{commit}" 2>/dev/null; then - commit_count=0 - while IFS= read -r commit; do - [ -n "${commit}" ] || continue - commit_count=$((commit_count + 1)) - message=$(git log -1 --format=%B "${commit}") - if ! printf '%s\n' "${message}" | awk ' - { line[NR]=$0 } - END { - n=NR - while (n > 0 && line[n] == "") n-- - if (n < 3 || line[n-1] != "" || line[n] !~ /^Authored-By: .+ \(.+\) $/) exit 1 - } - '; then - echo "FAIL[check-naming]: commit ${commit} must end with an Authored-By product/model trailer" - FAILED=1 - fi - done < <(git rev-list --reverse "${BASE}..${HEAD}" 2>/dev/null) + # Attribution belongs only in the PR description, not commit messages. + # Keep the existing nonempty-range gate independently of message content. + commit_count=$(git rev-list --count "${BASE}..${HEAD}" 2>/dev/null) || commit_count=0 if [ "${commit_count}" -eq 0 ]; then echo "FAIL[check-naming]: base-sha..head-sha contains no PR commits" FAILED=1 diff --git a/tests/run b/tests/run index da90bf1..a84a114 100755 --- a/tests/run +++ b/tests/run @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Tests for check.sh — exercises valid/invalid branch+title matrices. +# Tests for check.sh — exercises naming, PR attribution, and commit ranges. set -euo pipefail SCRIPT="$(cd "$(dirname "$0")/.." && pwd)/check.sh" @@ -98,6 +98,26 @@ check_chore_contract() { fi } +check_author_contract() { + local desc="$1" want_fail="$2" author="$3" body="$4" base="$5" head="$6" + local out rc=0 + out=$(cd "${FIXTURES}" && HEAD_BRANCH="chore/contract" \ + PR_TITLE="Align PR attribution policy" PR_AUTHOR="$author" \ + PR_BODY="$body" GITEA_SERVER_URL="https://code.test" GITEA_TOKEN="test-token" \ + GITEA_REPOSITORY="action/check-naming" GITEA_PR_NUMBER=1 PATH="${TEST_BIN}:$PATH" \ + BASE_SHA="$base" HEAD_SHA="$head" bash "$SCRIPT" 2>&1) || rc=$? + local got_lines=0 + printf '%s\n' "$out" | grep -qE '^FAIL' && got_lines=1 || true + if [ "$rc" -eq "$want_fail" ] && [ "$got_lines" -eq "$want_fail" ]; then + echo "PASS [$desc]" + PASS=$((PASS + 1)) + else + echo "FAIL [$desc]: expected exit=$want_fail fail-lines=$want_fail, got exit=$rc fail-lines=$got_lines" + echo " output: $out" + FAIL=$((FAIL + 1)) + fi +} + # break-glass check "dfritz exempt — invalid branch" 0 "totally/wrong-branch" "no prefix" "dfritz" check "dfritz exempt — mismatch" 0 "dev/bug-abc/thing" "[bug-xyz] Thing" "dfritz" @@ -233,20 +253,46 @@ else fi git -C "${FIXTURES}" commit --allow-empty -q -m "unwatermarked change" -BAD_HEAD=$(git -C "${FIXTURES}" rev-parse HEAD) -check_contract "agenthub 779 commits reject missing watermark" "$BODY" "$GOOD_HEAD" "$BAD_HEAD" "must end with an Authored-By" +PLAIN_HEAD=$(git -C "${FIXTURES}" rev-parse HEAD) +check_contract_pass "Bug-backed commits need no watermark" "$BODY" "$GOOD_HEAD" "$PLAIN_HEAD" check_chore_contract "chore requires PR attribution" "" "$BASE" "$GOOD_HEAD" "## Attribution must contain" -check_chore_contract "chore commits require watermark" "$(canonical_body bug-unused)" "$GOOD_HEAD" "$BAD_HEAD" "must end with an Authored-By" git -C "${FIXTURES}" commit --allow-empty -q -m $'misplaced trailer\nAuthored-By: Codex (GPT-5) ' MISPLACED_HEAD=$(git -C "${FIXTURES}" rev-parse HEAD) -check_contract "commit watermark requires blank separator" "$BODY" "$BAD_HEAD" "$MISPLACED_HEAD" "must end with an Authored-By" git -C "${FIXTURES}" commit --allow-empty -q -m "trailer not final" -m $'Authored-By: Codex (GPT-5) \nextra text' NONFINAL_HEAD=$(git -C "${FIXTURES}" rev-parse HEAD) -check_contract "commit watermark must be final trailer" "$BODY" "$MISPLACED_HEAD" "$NONFINAL_HEAD" "must end with an Authored-By" + +git -C "${FIXTURES}" commit --allow-empty -q -m "malformed legacy attribution" -m "Authored-By: incomplete" +MALFORMED_HEAD=$(git -C "${FIXTURES}" rev-parse HEAD) + +# Existing Human exemption is unchanged; other Humans and named Agents satisfy +# the same PR-description contract independently of any commit-message footer. +for author in dfritz human-contributor architect; do + for scenario in legacy plain misplaced nonfinal malformed; do + case "$scenario" in + legacy) head="$GOOD_HEAD" ;; + plain) head="$PLAIN_HEAD" ;; + misplaced) head="$MISPLACED_HEAD" ;; + nonfinal) head="$NONFINAL_HEAD" ;; + malformed) head="$MALFORMED_HEAD" ;; + esac + check_author_contract "$author accepts $scenario commit messages" 0 "$author" "$BODY" "$BASE" "$head" + done +done + +for author in human-contributor architect; do + check_author_contract "$author requires PR-description attribution" 1 "$author" "" "$BASE" "$PLAIN_HEAD" + check_author_contract "$author rejects a missing PR model" 1 "$author" \ + $'## Attribution\n- Authored-By: Codex ' "$BASE" "$PLAIN_HEAD" + check_author_contract "$author rejects a non-noreply PR address" 1 "$author" \ + $'## Attribution\n- Authored-By: Codex (GPT-5) ' "$BASE" "$PLAIN_HEAD" +done +check_author_contract "dfritz exemption still ignores PR attribution and commit range" 0 dfritz "" not-a-sha not-a-sha + check_contract "missing commit range rejects" "$BODY" "$GOOD_HEAD" "$GOOD_HEAD" "contains no PR commits" check_contract "invalid SHA rejects before git" "$BODY" "not-a-sha" "$GOOD_HEAD" "lowercase hexadecimal" +check_contract "unavailable commit rejects" "$BODY" 0000000000000000000000000000000000000000 "$GOOD_HEAD" "must name available commits" echo "" echo "Results: ${PASS} passed, ${FAIL} failed" -- 2.54.0