Skip to content
Merged
32 changes: 25 additions & 7 deletions .claude/skills/pr-flow/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,26 +95,38 @@ previous one, not all cut from `v2/main`.

**The `DCO` check fails the PR on any unsigned commit.** It is this repo's own
job (`.github/workflows/dco.yml` → `scripts/dco-check.mjs`, #2566), run on every
PR targeting `v2/main`, and it requires each commit to carry a `Signed-off-by: Name <email>`
v2 PR, stacked PRs included, and it requires each commit to carry a `Signed-off-by: Name <email>`
trailer whose name **and** email match either the commit's author or its
committer (case-insensitively). Its only exemptions are merge commits and
committer (case-insensitively). One relaxation: a commit GitHub itself
committed (`GitHub <noreply@github.com>`, as on a squash merge, whose author
name GitHub takes from the profile) passes on an author **email** match alone.
Its only exemptions are merge commits and
bot-authored commits; there is no partial credit — one unsigned commit out of six
fails the whole check, and the job's output names each offending commit and the
repair below.

⚠️ **It is a merge gate only because it is a _required_ status check** — a
ruleset setting, not something the workflow file can declare. The job runs on
`pull_request_target`, so its workflow is read from `main`: it reports on PRs
only once a milestone merge has carried it there (#2566). The probot DCO app
`pull_request`, from the PR's own ref, so it reports on every v2 PR — stacked
ones included, any `v2/**` base — with no wait for a
milestone merge (#2616). A second job, `DCO (v2/main push)`,
Comment thread
Copilot marked this conversation as resolved.
re-runs the check over every push that lands on `v2/main` — a backstop for
anything that merged without a passing PR check — so a red there means an
unsigned commit is already on the branch. The probot DCO app
it replaced was never required, so when the app was suspended its check simply
stopped appearing (after #1981) and nothing went red for two months. If the
`DCO` check is ever missing from a PR, treat that as the outage it is.

**Check before you push** — the same script runs locally against the range the
PR will show:
**Check before you push.** `npm run local:gate` already does it: its
`local:dco` stage runs the same script over `origin/v2/main..HEAD` (minus
anything already on `origin/main`), right after
`local:validate`. On a **stacked** branch that range covers the whole stack,
parents included, which is stricter than the child PR's own check (that one
runs against the parent's branch). To check on its own, against the range the
PR will show, pass the PR's actual base:

```sh
npm run dco:check -- --base origin/v2/main
npm run dco:check -- --base origin/v2/main # or origin/<parent branch> when stacked
```

**Prevent it with `git commit -s`.** Two things that look like automation and are
Expand All @@ -136,6 +148,12 @@ git rebase --rebase-merges --signoff origin/v2/main # the base the PR targets
git push --force-with-lease
```

⚠️ **On a stacked branch, rebase against the parent branch, never
`origin/v2/main`.** A `--signoff` rebase onto `origin/v2/main` rewrites every
parent commit too, which forks the child from its parent and breaks the stack.
If the unsigned commit is in the parent, repair the parent's own branch first,
then rebase the child onto the repaired parent.

`--rebase-merges` keeps any merge commit on the branch — without it the rebase
flattens them, silently dropping a conflict resolution that lives only in the
merge. Use `--force-with-lease` rather than `--force`, and only rewrite when you are the
Expand Down
17 changes: 17 additions & 0 deletions .claude/skills/pre-push-gate/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,23 @@ stale install otherwise passes every check and fails later as a behavioral test
reporting the *old* dependency's behavior as a product bug (#2494). Don't
"fix" that test.

### `local:dco`

A commit in `origin/v2/main..HEAD` (minus anything already on `origin/main`)
has no `Signed-off-by:` trailer matching its
author or committer (#2616). The output names each commit and prints the
repair. Repair and prevention (`git commit -s`, the `--signoff` rebase) are
step 3 of `/pr-flow`.

If it lists commits you never made, check which case you are in:

- **A stacked branch** (cut from another feature branch): `origin/v2/main..HEAD`
covers the whole stack, parent commits included. Fetching changes nothing.
An unsigned parent commit is repaired on the parent's own branch first, and
the child is then rebased onto the repaired parent (`/pr-flow` step 3).
- **Not stacked**: your `origin/v2/main` is probably behind what the branch was
cut from. `git fetch origin v2/main` and re-run.

### `verify:action-pins`

A job that holds a credential (`id-token`/`packages: write`, a non-default
Expand Down
131 changes: 100 additions & 31 deletions .github/workflows/dco.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,48 +3,74 @@
# The app was suspended and its check silently stopped appearing after #1981
# (2026-08-12). Nothing went red, because it was never a required check — so
# for two months "the DCO check is a hard merge gate" was true only by habit.
# This job is the repo-owned replacement: `scripts/dco-check.mjs` fails the PR
# unless every commit in it carries a `Signed-off-by:` matching its author or
# committer (merge and bot-authored commits exempt — the app's rule).
# This workflow is the repo-owned replacement: `scripts/dco-check.mjs` fails
# unless every commit in a range carries a `Signed-off-by:` matching its
# author or committer (merge and bot-authored commits exempt — the app's rule).
#
# ⚠️ A job only GATES when its check is required. Making `DCO` a required
# status check is a ruleset change in repo settings (one covering `v2/main`
# does not exist yet), not something a workflow file can do — without it, this
# job going missing would pass as silently as the app did.
# Two jobs, one script:
#
# Its own workflow because it needs the PR's base, which `main.yml` (push
# only) never sees.
# - `DCO` — on every v2 PR, over the PR's commits. This is the
# one to make a REQUIRED status check (a ruleset change in repo settings,
# not something this file can do — without it, the job going missing
# passes as silently as the app did).
# - `DCO (v2/main push)` — a backstop over every push that lands on
# `v2/main`, for whatever reached the branch without a passing PR check
# while this file and the script stayed intact: an admin merge, a direct
# push, a merge made while the PR check was red or missing. NOT a PR that
# edited the check itself (below).
# It reports when the commit lands, not one milestone later — it cannot
# un-merge anything, but an unsigned commit found the same day is still
# cheap to deal with.
#
# ⚠️ `pull_request_target`, deliberately — a check that gates a PR must not run
# code the PR can change. Under `pull_request` both this file and the script
# come from the PR's merge ref, so a PR could rewrite either to pass itself.
# `pull_request_target` takes the workflow from the repo's DEFAULT branch
# (`main`), and the job checks out the PR's base (`v2/main`) for the script —
# both already-merged code. The PR's commits are only FETCHED: `git log` reads
# their metadata and nothing from the PR is ever executed, which is the
# condition that makes `pull_request_target` safe. Keep it that way: never
# check out, install, build or run the PR head in this job.
# ⚠️ `pull_request`, not `pull_request_target` (#2616). #2603 shipped on
# `pull_request_target` so a PR could not rewrite its own check to pass — but
# GitHub reads `pull_request_target` workflows from the DEFAULT branch
# (`main`), so the check reported on no v2 PR at all until a milestone merge
# carried this file there, and unsigned commits would have surfaced only when
# the milestone PR into `main` was opened. Under `pull_request` the workflow
# runs from the PR's own ref and works the moment it is on `v2/main`, and an
# edit to this file takes effect on the PR that makes it.
#
# ⚠️ Rollout follows from the workflow living on `main`: this file does
# nothing until a milestone merge carries it from `v2/main` to `main`, and a
# later edit to it takes effect only on the next milestone merge. An edit to
# `scripts/dco-check.mjs` takes effect as soon as it merges to `v2/main`. Make
# `DCO` a required check only once this file is on `main` — required before
# then, every v2 PR would wait on a check that never reports.
# The trade-off, accepted deliberately: a PR can now edit this file to pass
# itself. That is tolerable here because PRs are opened by maintainers only
# (AGENTS.md, Contributing) and the check exists to catch a FORGOTTEN signoff,
# not a forged one — the trailer is self-asserted text either way (see the
# header of `scripts/dco-check.mjs`). ⚠️ The push job does NOT cover that
# case: a `push` run also reads this file, and runs the script, from the
# pushed revision — so a PR that edits the check edits the backstop with it.
# Nothing in a workflow the repo's own branches carry can close that; only a
# check read from elsewhere could, which is what `pull_request_target` was and
# what this change gives up. The control for it is review: an edit to this
# file or to `scripts/dco-check.mjs` is in the PR's diff. The PR job keeps the
# trigger safe anyway: it checks out the PR's BASE for the script and only
# FETCHES the PR's commits, reading their metadata with `git log` — nothing
# from the PR is checked out, installed, built or run.
#
# Scoped to `v2/main`. Once on `main` an unscoped trigger would also fire for
# `v1/main` PRs (whose checkout has no script) and for milestone PRs into
# `main` (whose range holds GitHub's squash-merge commits, signed under a
# different name). `edited` re-runs it when a PR is retargeted, since the
# checked range changes with the base even when the head does not.
# The PR job runs on every `v2/**` base — `v2/main` and, since branch names
# carry their version segment, every v2 feature branch — so a stacked PR is
# checked on its own range rather than only once its parent merges (the
# servers repo's #5055 checks stacks too). A positive filter rather than a
# list of exclusions, so the scope is the v2 line by construction: `main`
# (milestone PRs carry GitHub's squash-merge commits and reach back before the
# check existed) and the whole v1 line, `v1/main` and its stacks alike (no copy
# of the script), are out of it without being named. A stacked PR's base must
# be a branch cut after #2603, since the script is read from it. `edited` re-runs the PR job when a PR is retargeted,
# since the checked range changes with the base even when the head does not.
#
# Before either job, `npm run local:gate` runs the same script over
# `origin/v2/main..HEAD`, minus anything already on `origin/main` so it holds
# on a milestone-merge branch too (`local:dco`), so an unsigned commit is normally
# caught before it is pushed at all.
#
# Only `contents: read`, no credential persisted, no secret — so it stays on
# moving action tags (#2235; see verify:action-pins, #2484).
name: DCO

on:
pull_request_target:
pull_request:
types: [opened, synchronize, reopened, edited]
branches: ['v2/**']
push:
branches: [v2/main]

permissions:
Expand All @@ -53,14 +79,21 @@ permissions:
jobs:
dco:
name: DCO
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
# A newer push to the same PR supersedes the range being checked. Job-level
# and PR-keyed on purpose: the push job below must NOT share it, since a
# cancelled push run is a range that is never checked.
concurrency:
group: dco-pr-${{ github.event.pull_request.number }}
cancel-in-progress: true
# A git fetch, a git log and a node script; expected to finish well inside
# a minute. Not yet observed on a runner — revisit with the measured range
# per the rule in main.yml (#2333) once it has a history.
timeout-minutes: 5

steps:
- name: Checkout the base branch (trusted) with full history
- name: Checkout the base branch with full history
uses: actions/checkout@v7
with:
ref: ${{ github.base_ref }}
Expand All @@ -87,3 +120,39 @@ jobs:
BASE_REF: ${{ github.base_ref }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
run: node scripts/dco-check.mjs --base "origin/$BASE_REF" --head "$HEAD_SHA"

dco-push:
name: DCO (v2/main push)
if: github.event_name == 'push'
runs-on: ubuntu-latest
# Same work as the PR job; same unmeasured budget.
timeout-minutes: 5

steps:
- name: Checkout the pushed commit with full history
uses: actions/checkout@v7
Comment thread
Copilot marked this conversation as resolved.
with:
fetch-depth: 0
persist-credentials: false

- name: Setup Node.js
uses: actions/setup-node@v7
with:
node-version: '22.x'

- name: Check every pushed commit is signed off
# `before..after` is exactly what the push added: for a PR merge, the
# merge commit (exempt) plus every commit it brought in. A zero
# `before` means the branch was just created, which leaves no range to
# check. A `before` missing from the history (a force-push that
# rewrote it away) makes `git log` fail and the script exit 2 — loud
# on purpose, since `v2/main` is never meant to be rewritten.
env:
BEFORE: ${{ github.event.before }}
AFTER: ${{ github.event.after }}
run: |
if [ "$BEFORE" = "0000000000000000000000000000000000000000" ]; then
echo "Branch creation push: no prior commit, nothing to check."
exit 0
fi
node scripts/dco-check.mjs --base "$BEFORE" --head "$AFTER"
Loading
Loading