From 1a667b384a214e2a9851a761ee0ec2a507c76e55 Mon Sep 17 00:00:00 2001 From: Frank Steiler Date: Wed, 8 Jul 2026 02:12:55 +0200 Subject: [PATCH] ci(trailers): enforce agent trailer preservation at commit, merge, and CI layers Fixes #1820 Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) --- .claude/agents/dev-team-lead.md | 16 ++++-- .claude/skills/dependabot/SKILL.md | 12 ++++- .claude/skills/develop/SKILL.md | 29 ++++++----- .claude/skills/epic-close/SKILL.md | 15 +++++- .claude/skills/fix-e2e/SKILL.md | 18 +++++-- .claude/skills/release/SKILL.md | 2 +- .claude/skills/review-pr/SKILL.md | 2 +- .github/workflows/ci.yml | 21 +++++++- CLAUDE.md | 55 ++++++++++++++++++++ scripts/check-trailers.sh | 83 ++++++++++++++++++++++++++++++ 10 files changed, 227 insertions(+), 26 deletions(-) create mode 100755 scripts/check-trailers.sh diff --git a/.claude/agents/dev-team-lead.md b/.claude/agents/dev-team-lead.md index ce038e369..8175f8316 100644 --- a/.claude/agents/dev-team-lead.md +++ b/.claude/agents/dev-team-lead.md @@ -327,7 +327,15 @@ VERDICT: CHANGES_REQUIRED ## Commit & Push Details (Mode: commit) 1. Stage all changes: `git add ` (prefer specific files over `git add -A`) -2. Commit with conventional commit message and Co-Authored-By trailers for **all contributing agents**: + +2. **Determine the required trailer set — do not trust the orchestrator's contributing-agents list as the sole source.** Historically, commits have landed on `beta` with only the `dev-team-lead` trailer despite `server/`/`client/` files clearly having been changed by an implementing agent (issue #1820: 7 of 11 non-infra commits in one batch alone). To prevent this: + + a. Run `git diff --name-only --cached` against the staged files from step 1. + b. Classify them against CLAUDE.md's Delegation Enforcement rules 2-6 (backend-developer for `server/`/`shared/` non-test; frontend-developer for `client/` non-i18n-de/non-glossary/non-test; translator for `client/src/i18n/de/` + `glossary.json`; e2e-test-engineer for `e2e/`; qa-integration-tester for co-located `*.test.ts`/`*.test.tsx` outside `e2e/`). + c. Union this rule-derived set with the orchestrator's passed-in "Contributing agents list." If the rule-derived set names an agent the orchestrator did **not** list, include that agent's trailer anyway — the file diff is ground truth, not the orchestrator's memory of which agents it launched — and say so explicitly in your response to the orchestrator (e.g., "Note: added `backend-developer` trailer — `server/` files changed but this agent wasn't in your Contributing agents list; verify it was actually launched for this work"). Never silently omit a trailer the diff requires. + d. Always include your own `dev-team-lead` trailer plus every agent from the union in (c). + +3. Commit with conventional commit message and the trailers determined in step 2: ``` feat(scope): description @@ -341,9 +349,9 @@ VERDICT: CHANGES_REQUIRED Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) ``` - Include only the trailers for agents that actually contributed, using the exact strings from CLAUDE.md's Canonical Agent Trailers table. Use `feat(scope):` for stories, `fix(scope):` for bugs. + Use the **exact strings** from CLAUDE.md's Canonical Agent Trailers table — never a bare model name with no agent name, never a lowercase `co-authored-by`. Use `feat(scope):` for stories, `fix(scope):` for bugs. -3. Push: `git push -u origin ` +4. Push: `git push -u origin ` There is no pre-commit hook. Local validation (lint:fix/format/lint) is each implementing agent's responsibility per CLAUDE.md's Local Validation Policy, verified by you during `[MODE: review]`. Full validation (test, typecheck, build, audit) runs as CI's Quality Gates job after push. If CI Quality Gates fail: @@ -368,7 +376,7 @@ Fixes # - [ ] CI Quality Gates pass (typecheck, tests, build, audit) Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) - + EOF )" ``` diff --git a/.claude/skills/dependabot/SKILL.md b/.claude/skills/dependabot/SKILL.md index 7d49c2de0..2a6bbecdd 100644 --- a/.claude/skills/dependabot/SKILL.md +++ b/.claude/skills/dependabot/SKILL.md @@ -150,10 +150,18 @@ For each `READY` PR with no `BREAKING` or `BLOCKING` findings from step 3: if [ "$state" != "MERGEABLE" ]; then echo "PR is not mergeable (state: $state) — surfacing to user"; continue; fi ``` -3. Squash-merge: +3. Squash-merge, rebuilding the body per CLAUDE.md's **Squash-Merge Trailer Preservation** pattern (the `TRAILERS` variable will be empty for PRs merged as-is with no agent fix commits — that's fine, the body is just omitted; only PRs that went through step 5's fix loop will have agent trailers to preserve): ```bash - gh pr merge --repo steilerDev/cornerstone --squash + BASE_BRANCH=beta + TRAILERS=$(git log origin/${BASE_BRANCH}..HEAD --format="%b" | grep -iE '^co-authored-by:' | sed -E 's/^[Cc]o-[Aa]uthored-[Bb]y:/Co-Authored-By:/' | sort -u) + BODY="$(cat < --repo steilerDev/cornerstone --squash --subject "" --body "$BODY" ``` 4. Mark the PR's task `completed` and record it for the final report. Continue to the next PR. diff --git a/.claude/skills/develop/SKILL.md b/.claude/skills/develop/SKILL.md index 98b0b5ca5..0aa59a2cb 100644 --- a/.claude/skills/develop/SKILL.md +++ b/.claude/skills/develop/SKILL.md @@ -269,18 +269,10 @@ The dev-team-lead stages files, commits with conventional message + all agent tr After the commit is created, verify that commit trailers match the agents launched: ```bash -git log origin/beta..HEAD --format="%b" +bash scripts/check-trailers.sh origin/beta HEAD ``` -If production files were changed (`git diff --name-only origin/beta..HEAD | grep -E '^(server|client|shared)/'`), verify the commit body contains the appropriate Co-Authored-By trailers: - -- Files under `server/` or `shared/`, excluding `*.test.ts`/`*.test.tsx` → must have `Co-Authored-By: Claude backend-developer (Haiku` -- Files under `client/` (except `client/src/i18n/de/`, `client/src/i18n/glossary.json`, and `*.test.ts`/`*.test.tsx`) → must have `Co-Authored-By: Claude frontend-developer (Haiku` -- Files under `client/src/i18n/de/` or `client/src/i18n/glossary.json` → must have `Co-Authored-By: Claude translator (Sonnet` -- Files under `e2e/` → must have `Co-Authored-By: Claude e2e-test-engineer (Sonnet` -- Files matching `*.test.ts` or `*.test.tsx` outside `e2e/` → must have `Co-Authored-By: Claude qa-integration-tester (Sonnet` - -If trailers are missing, the dev-team-lead missed an agent in the contributing list. Re-launch `[MODE: commit]` with the corrected list. +If this fails, the dev-team-lead missed an agent in the contributing list despite the Layer 0 self-check in its `[MODE: commit]` process (see `.claude/agents/dev-team-lead.md`). Re-launch `[MODE: commit]` with the corrected list — the CI `trailer-check` job will also catch this if it slips through. ### 7. Verify PR @@ -411,10 +403,21 @@ In multi-item mode, present a **per-item summary table**: | #61 | Add export button to Gantt | Resolved | ``` -Once CI is green and all reviewers have approved, merge to beta: +Once CI is green and all reviewers have approved, merge to beta. Rebuild the squash body from the branch's commit trailers per CLAUDE.md's **Squash-Merge Trailer Preservation** canonical pattern: -``` -gh pr merge --squash +```bash +BASE_BRANCH=beta +SUBJECT="" +TRAILERS=$(git log origin/${BASE_BRANCH}..HEAD --format="%b" | grep -iE '^co-authored-by:' | sed -E 's/^[Cc]o-[Aa]uthored-[Bb]y:/Co-Authored-By:/' | sort -u) +BODY="$(cat < + +Fixes # # one line per issue in multi-item mode + +${TRAILERS} +EOF +)" +gh pr merge --squash --subject "$SUBJECT" --body "$BODY" ``` If the user reports issues with a merged PR, take the user's feedback as new input and start a new `/develop` cycle to address it. diff --git a/.claude/skills/epic-close/SKILL.md b/.claude/skills/epic-close/SKILL.md index feef7899a..399757087 100644 --- a/.claude/skills/epic-close/SKILL.md +++ b/.claude/skills/epic-close/SKILL.md @@ -98,7 +98,20 @@ If there are refinement items to address: gh pr create --base beta --title "chore: address refinement items for epic #" --body "..." ``` 8. **Wait 5 seconds** after creating the PR, then check mergeability: `gh pr view --repo steilerDev/cornerstone --json mergeable -q '.mergeable'`. **Only continue if the result is `MERGEABLE`.** If `CONFLICTING`, rebase onto `beta`, force-push, and re-check. If `UNKNOWN`, wait a few more seconds and retry. Once mergeability is confirmed, wait for CI using the **CI Gate Polling** pattern from `CLAUDE.md` (beta variant — wait for `Quality Gates`) -9. Squash merge: `gh pr merge --squash ` +9. Squash merge, rebuilding the body per CLAUDE.md's **Squash-Merge Trailer Preservation** pattern: + + ```bash + BASE_BRANCH=beta + SUBJECT="chore: address refinement items for epic #" + TRAILERS=$(git log origin/${BASE_BRANCH}..HEAD --format="%b" | grep -iE '^co-authored-by:' | sed -E 's/^[Cc]o-[Aa]uthored-[Bb]y:/Co-Authored-By:/' | sort -u) + BODY="$(cat < + + ${TRAILERS} + EOF + )" + gh pr merge --squash --subject "$SUBJECT" --body "$BODY" + ``` If no refinement items exist, skip to step 5. diff --git a/.claude/skills/fix-e2e/SKILL.md b/.claude/skills/fix-e2e/SKILL.md index 5f56128a6..48d23ba92 100644 --- a/.claude/skills/fix-e2e/SKILL.md +++ b/.claude/skills/fix-e2e/SKILL.md @@ -133,11 +133,13 @@ fix(e2e):
-Co-Authored-By: Claude () +Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) EOF )" ``` +Use the exact per-agent string from CLAUDE.md's **Canonical Agent Trailers** table — this commit is authored directly by this skill (not via `dev-team-lead [MODE: commit]`), so there is no automated normalization; get the casing and model string right the first time. If `backend-developer` or `frontend-developer` also contributed a fix (per step 3's classification table), include their canonical trailers too. + #### 4b. Push and create/update PR ```bash @@ -211,10 +213,20 @@ After CI completes: case "$bucket" in pass) echo "Quality Gates passed"; break ;; fail) echo "Quality Gates FAILED"; exit 1 ;; *) sleep 30 ;; esac done ``` -2. Squash merge the PR: +2. Squash merge the PR, rebuilding the body per CLAUDE.md's **Squash-Merge Trailer Preservation** pattern: + ```bash - gh pr merge --squash --repo steilerDev/cornerstone + BASE_BRANCH=beta + TRAILERS=$(git log origin/${BASE_BRANCH}..HEAD --format="%b" | grep -iE '^co-authored-by:' | sed -E 's/^[Cc]o-[Aa]uthored-[Bb]y:/Co-Authored-By:/' | sort -u) + BODY="$(cat < + + ${TRAILERS} + EOF + )" + gh pr merge --squash --repo steilerDev/cornerstone --subject "" --body "$BODY" ``` + 3. Mark all tasks completed. Report success to the user with: - Total iterations needed - Summary of all fixes applied diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 68f51107e..5ccfa0773 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -306,7 +306,7 @@ Commit documentation updates to `beta` via a PR: gh pr create --base beta --title "docs: update documentation for release" --body "..." ``` -Wait for CI, then squash merge. +Wait for CI, then squash merge. Rebuild the squash body per CLAUDE.md's **Squash-Merge Trailer Preservation** pattern (`SUBJECT="docs: update documentation for release"`, base branch `beta`) — `docs-writer`'s own trailer is the only one expected here, but the pattern still guards against GitHub dropping it on a single-commit PR. **Note:** Documentation runs after user approval (step 4) to ensure docs reflect the final state, including any changes from feedback rounds. diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md index 0eb8ee5e2..20d989ec8 100644 --- a/.claude/skills/review-pr/SKILL.md +++ b/.claude/skills/review-pr/SKILL.md @@ -121,7 +121,7 @@ Present to the user: 2. **Independent Behavior Change Summary** — read the diff stored from Step 1 and describe what actually changed in user-visible terms, independent of the PR description or changelog. Flag any discrepancies between what the PR claims to do and what the code actually does. 3. **CI Status** — pass/fail for the required gate check(s) per CLAUDE.md: `Quality Gates` (required on both `beta`- and `main`-targeted PRs), plus `E2E Gates` (required only when the PR targets `main`). `Docker`, `Docker PR Release`, and `Merge E2E Reports` run but are not required gates — report them only if directly relevant, not as blocking checks. "Skipped" if review was blocked. 4. **Overall Verdict** — `APPROVED` or `BLOCKED` with specific next steps: - - If approved: user can merge at their discretion (`gh pr merge --squash `) + - If approved: user can merge at their discretion (`gh pr merge --squash `; for a single-commit PR, consider rebuilding the body per CLAUDE.md's **Squash-Merge Trailer Preservation** pattern if the PR carries agent trailers you want preserved) - If blocked: list what the contributor needs to fix, suggest re-running `/review-pr ` after fixes are pushed **The orchestrator never merges.** The user decides when to merge. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 18cbb8261..ae56923aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,6 +69,24 @@ jobs: - name: Build run: npm run build + trailer-check: + name: Trailer Check + runs-on: ubuntu-latest + needs: [detect-changes] + if: needs.detect-changes.outputs.app == 'true' && github.event.pull_request.user.login != 'dependabot[bot]' + + steps: + - name: Checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + + - name: Verify agent trailers + run: | + BASE_REF="${{ github.base_ref }}" + if [ -z "$BASE_REF" ]; then BASE_REF="main"; fi + bash scripts/check-trailers.sh "origin/${BASE_REF}" "HEAD" + test: name: Test (Shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }}) runs-on: ubuntu-latest @@ -153,7 +171,7 @@ jobs: quality-gates: name: Quality Gates runs-on: ubuntu-latest - needs: [static-analysis, test, docker, e2e-warmup, e2e-smoke] + needs: [static-analysis, trailer-check, test, docker, e2e-warmup, e2e-smoke] if: ${{ !cancelled() }} steps: - name: Check results @@ -173,6 +191,7 @@ jobs: } check_job "static-analysis" "${{ needs.static-analysis.result }}" + check_job "trailer-check" "${{ needs.trailer-check.result }}" check_job "test" "${{ needs.test.result }}" check_job "docker" "${{ needs.docker.result }}" check_job "e2e-warmup" "${{ needs.e2e-warmup.result }}" diff --git a/CLAUDE.md b/CLAUDE.md index 3b3b7cda5..2ee012fd5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -273,6 +273,61 @@ Detect: head commit's first line literally contains `[skip ci]`; `gh run list -- **Recovery if it already happened:** push another commit to `beta` via a fresh tiny PR with a clean title (e.g., `chore: retrigger promotion CI`). That advances HEAD with a clean message, fires `pull_request:synchronize` on the promotion PR, and CI runs normally. +### Squash-Merge Trailer Preservation (canonical pattern) + +GitHub's default `--squash` merge body varies with commit count and shape — for single-commit +branches it may reuse the sole commit's body verbatim (so a commit missing a trailer stays +missing); for multi-commit branches it concatenates all commit bodies (which tends to preserve +trailers but also duplicates them, once per near-identical casing variant). Never rely on the +default. Every skill that squash-merges a PR carrying agent trailers must rebuild the merge +commit's subject and body explicitly: + +```bash +BASE_BRANCH=beta # or the PR's actual base +SUBJECT="(): " # clean conventional title -- must not contain a literal + # "[skip ci]" or equivalent (see CI Skip-Directive Quirks) +TRAILERS=$(git log origin/${BASE_BRANCH}..HEAD --format="%b" \ + | grep -iE '^co-authored-by:' \ + | sed -E 's/^[Cc]o-[Aa]uthored-[Bb]y:/Co-Authored-By:/' \ + | sort -u) +BODY="$(cat < + +Fixes # + +${TRAILERS} +EOF +)" +gh pr merge --squash --subject "$SUBJECT" --body "$BODY" +``` + +This normalizes the `Co-Authored-By:` label casing before deduplication, so `Co-Authored-By:` and +`co-authored-by:` variants of the same line collapse to one. It does **not** merge a bare-model +trailer (e.g. `Claude Sonnet 4.6` with no agent name) with a properly-named one — those are a +distinct, pre-existing format-drift problem that Squash-Merge Trailer Preservation cannot repair +after the fact. Going forward, `dev-team-lead [MODE: commit]` always writes the canonical +per-agent string from the Canonical Agent Trailers table, and `scripts/check-trailers.sh` (see +below) fails the PR before merge if a required agent's canonical name is absent — so bare-model +trailers should not recur as the _only_ record of an agent's contribution. + +**Note**: this only applies to squash merges carrying agent trailers. It does not apply to the +`beta` → `main` promotion merge (`gh pr merge --merge`), which preserves individual commits (and +their trailers) natively by design — see the Release Model table above. + +### Trailer Verification Script + +`scripts/check-trailers.sh ` is the single source of truth for "does this +commit range carry the trailers CLAUDE.md's Delegation Enforcement rules require." It is used in +two places: + +1. **`/develop` step 6h / step 9** (orchestrator, before merging) — run it directly instead of + hand-checking with grep. +2. **CI's `trailer-check` job** (automated, on every PR touching production paths) — see + `.github/workflows/ci.yml`. + +Detection inside the script is case-insensitive; writing trailers is always canonical-case per the +table above. + ### GitHub Rate-Limit Retry Policy When `gh` or `git push` commands fail with a GitHub rate-limit error (primary API limit, secondary abuse limit, or `HTTP 403`/`HTTP 429` with a rate-limit message), retry with **exponential backoff** instead of aborting: diff --git a/scripts/check-trailers.sh b/scripts/check-trailers.sh new file mode 100755 index 000000000..0085e7d81 --- /dev/null +++ b/scripts/check-trailers.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# check-trailers.sh +# +# Verifies that commits in a given range carry the Co-Authored-By agent +# trailers required by CLAUDE.md's "Delegation Enforcement" rules (2-6), +# based on which paths changed in that range. +# +# Usage: check-trailers.sh +# +# Skip condition (exit 0, no checks run): the range contains NO +# "Claude ... " trailer at all. That's the +# signal this range is human-authored / outside the agent workflow -- +# agent trailer rules don't apply to humans. (Dependabot PRs are skipped +# one level up, at the CI job's `if:`, by PR author login.) +# +# Detection is case-insensitive (format drift like "co-authored-by" vs +# "Co-Authored-By" must still be caught as present). Writing trailers is +# always canonical-case per CLAUDE.md's Canonical Agent Trailers table -- +# this script only reads. + +set -euo pipefail + +BASE_REF="${1:?usage: check-trailers.sh }" +HEAD_REF="${2:?usage: check-trailers.sh }" + +CHANGED=$(git diff --name-only "${BASE_REF}...${HEAD_REF}") +TRAILERS=$(git log "${BASE_REF}..${HEAD_REF}" --format="%B" | grep -iE '^co-authored-by:' || true) + +# Human-authored range: no Claude agent trailers anywhere -- skip entirely. +if ! echo "$TRAILERS" | grep -qiE 'claude[[:space:]]+[a-z0-9-]+.*'; then + echo "check-trailers: no Claude agent trailers found in range -- treating as human-authored, skipping." + exit 0 +fi + +FAILED=0 + +require() { + local agent="$1" label="$2" + if ! echo "$TRAILERS" | grep -qiE "claude[[:space:]]+${agent}[[:space:]]*\("; then + echo "ERROR: files matching '${label}' changed but no 'Co-Authored-By: Claude ${agent} (...)' trailer found in range ${BASE_REF}..${HEAD_REF}." >&2 + FAILED=1 + fi +} + +# Rule 2: server/ or shared/, excluding co-located tests -> backend-developer +BACKEND_NONTEST=$(echo "$CHANGED" | grep -E '^(server|shared)/' | grep -vE '\.test\.tsx?$' || true) +if [[ -n "$BACKEND_NONTEST" ]]; then + require "backend-developer" "server/**, shared/** (non-test)" +fi + +# Rule 3: client/, excluding i18n/de/, glossary.json, and co-located tests -> frontend-developer +FRONTEND_NONTEST=$(echo "$CHANGED" | grep -E '^client/' \ + | grep -vE '^client/src/i18n/de/|^client/src/i18n/glossary\.json$' \ + | grep -vE '\.test\.tsx?$' || true) +if [[ -n "$FRONTEND_NONTEST" ]]; then + require "frontend-developer" "client/** (non-i18n-de, non-glossary, non-test)" +fi + +# Rule 4: client/src/i18n/de/ or glossary.json -> translator +TRANSLATOR_FILES=$(echo "$CHANGED" | grep -E '^client/src/i18n/de/|^client/src/i18n/glossary\.json$' || true) +if [[ -n "$TRANSLATOR_FILES" ]]; then + require "translator" "client/src/i18n/de/**, glossary.json" +fi + +# Rule 5: e2e/ -> e2e-test-engineer +E2E_FILES=$(echo "$CHANGED" | grep -E '^e2e/' || true) +if [[ -n "$E2E_FILES" ]]; then + require "e2e-test-engineer" "e2e/**" +fi + +# Rule 6: co-located unit/integration tests outside e2e/ -> qa-integration-tester +QA_FILES=$(echo "$CHANGED" | grep -vE '^e2e/' | grep -E '\.test\.tsx?$' || true) +if [[ -n "$QA_FILES" ]]; then + require "qa-integration-tester" "*.test.ts(x) outside e2e/" +fi + +if [[ $FAILED -ne 0 ]]; then + echo "" >&2 + echo "See CLAUDE.md's 'Delegation Enforcement' and 'Canonical Agent Trailers' sections." >&2 + exit 1 +fi + +echo "check-trailers: all required agent trailers present for ${BASE_REF}..${HEAD_REF}."