Skip to content

Commit 5e3eb8f

Browse files
committed
feat(ci): refuse card relations in commit messages at push, not on the PR
The rule that no commit message carries a card relation was enforced on the pull request, over its commit list. By then the cheap repair was gone: a new commit on top joins that list, and removing the message is the history rewrite this repo forbids, so two rounds each paid a full redo. - `scripts/check-commit-card-trailers.mjs` judges exactly the commits a push would publish — everything it sees is unpublished, so its remedy is an ordinary reword. It adds no grammar: the extractors are the sweep's, and the trailer-position rule covers a bare reference with no keyword beside it. - `.githooks/pre-push` calls it after the os-regen deferral check, which keeps its behaviour unchanged; the ref lines git writes are now read once and handed to the check whose input they are. - `check:partof-closing-keyword` drops its commit-list rule: the squash message is taken from the PR body now, so a branch commit's trailer never lands. Claude-Session: https://claude.ai/code/session_01MCLBsUgfykL74aU716rzVK Co-authored-by: Claude <noreply@anthropic.com>
1 parent cec7d5b commit 5e3eb8f

6 files changed

Lines changed: 948 additions & 748 deletions

File tree

‎.githooks/pre-push‎

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,39 @@
11
#!/bin/sh
22
# Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
33
#
4-
# The collection half of the os-regen deferred merge (#8047). `pre-commit` may
5-
# accept a MERGE commit whose regeneration is deferred to the next commit — but
6-
# at the moment it decides, that next commit does not exist yet, so it can only
7-
# RECORD the deferral. Something has to be the event that notices the deferral
8-
# was never discharged. Only two events can follow a merge commit: another
9-
# commit (which `pre-commit` already refuses while the artifacts are stale) and
10-
# the push. This is the push.
4+
# TWO refusals, one event. Both are about something that is cheap to repair now
5+
# and expensive — or impossible — the moment the push lands.
6+
#
7+
# 1. The collection half of the os-regen deferred merge (#8047). `pre-commit`
8+
# may accept a MERGE commit whose regeneration is deferred to the next commit —
9+
# but at the moment it decides, that next commit does not exist yet, so it can
10+
# only RECORD the deferral. Something has to be the event that notices the
11+
# deferral was never discharged. Only two events can follow a merge commit:
12+
# another commit (which `pre-commit` already refuses while the artifacts are
13+
# stale) and the push. This is the push.
14+
#
15+
# 2. Card relations in commit messages. The relation is declared ONCE, in the PR
16+
# body; a commit carrying it is refused here rather than on the pull request,
17+
# because on a PUSHED branch nothing an author may legally do removes the
18+
# message — the check that used to report it read the PR's commit list, so a new
19+
# commit on top joins that list, and two rounds each paid a full redo. Before
20+
# the push, the repair is an ordinary reword. The script's header is the
21+
# authority; this file is the invocation.
1122
#
1223
# Registered by the same `core.hooksPath=.githooks` that registers `pre-commit`,
1324
# so it needs no change to `scripts/setup-git-hooks.mjs` and no separate opt-in.
1425
#
15-
# Cheap by construction, exactly like `pre-commit`: with no pending marker the
16-
# script exits before doing any work, which is every push in this repo that did
17-
# not just defer a merge. The ref list git writes on stdin is drained and
18-
# ignored on purpose — the marker is per-worktree state, not per-ref state, so
19-
# which refs are being pushed cannot change the answer.
26+
# The ref list git writes on stdin is read ONCE, here, and handed to the check
27+
# that needs it. The regen marker is per-worktree state rather than per-ref
28+
# state, so which refs are being pushed cannot change that answer; the card
29+
# check is the opposite — the refs are its whole input, since they say which
30+
# commits this push would publish.
31+
32+
REFS=$(cat)
33+
ROOT=$(git rev-parse --show-toplevel)
2034

2135
if [ -z "$OS_SKIP_REGEN_CHECK" ]; then
22-
cat >/dev/null
23-
node "$(git rev-parse --show-toplevel)/scripts/check-regen-pending.mjs" --pre-push || exit 1
36+
node "$ROOT/scripts/check-regen-pending.mjs" --pre-push || exit 1
2437
fi
38+
39+
printf '%s\n' "$REFS" | node "$ROOT/scripts/check-commit-card-trailers.mjs" --pre-push || exit 1

‎.github/workflows/lint.yml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -982,6 +982,21 @@ jobs:
982982
- name: Part-of closing-keyword guard self-test
983983
run: pnpm check:partof-closing-keyword
984984

985+
# Commit card-trailer refusal self-test. The check itself is a PRE-PUSH
986+
# hook — it judges the commits a push would publish, which no CI job has
987+
# and no CI job should try to reconstruct — so what runs HERE is its
988+
# self-test, the half whose verdict depends on nothing but the script.
989+
# Unconditional for the same reason as the step above.
990+
#
991+
# It is the only thing standing behind that hook: a hook is registered per
992+
# clone, runs on a developer's machine, and reports to nobody, so a break
993+
# in its rule is invisible everywhere else. The self-test also pins the
994+
# WIRING (the hook still calls the script, and still runs the os-regen
995+
# deferral check it carried before), so unwiring it reddens here rather
996+
# than going quiet. Pure functions plus two file reads; ~0.1s.
997+
- name: Commit card-trailer pre-push refusal self-test
998+
run: pnpm check:commit-card-trailers
999+
9851000
# Publish-smoke tarball pin-set self-test. The assertion it pins lives on
9861001
# the RELEASE path (scripts/publish-smoke-pack.mjs runs only inside the
9871002
# packed-tarball smoke), so without this step a regression in it would be

‎.github/workflows/partof-closing-keyword-guard.yml‎

Lines changed: 7 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@ on:
3333
pull_request:
3434
types: [opened, edited, reopened, synchronize]
3535

36-
# `contents: read` checks the repo out to get at the script. `pull-requests:
37-
# read` is what the commit-list gather below needs, and naming a `permissions:`
38-
# block at all sets every scope NOT listed to `none`, so both must be spelled.
39-
# Read-only is the whole grant: this gate reports, and never closes a PR,
40-
# comments, or edits a body.
36+
# `contents: read` checks the repo out to get at the script, and naming a
37+
# `permissions:` block at all sets every scope NOT listed to `none`, so that one
38+
# line is the whole grant. It used to also grant `pull-requests: read`, for a
39+
# step that gathered the PR's commit messages; that rule is a pre-push hook now
40+
# (the script header says why), so the scope went with it. This gate reports,
41+
# and never closes a PR, comments, or edits a body.
4142
permissions:
4243
contents: read
43-
pull-requests: read
4444

4545
concurrency:
4646
group: partof-closing-keyword-${{ github.event.pull_request.number }}
@@ -93,47 +93,8 @@ jobs:
9393
# No install step: the script imports one sibling module and reads no
9494
# workspace package, so `node` on the pinned runtime is the whole
9595
# toolchain it needs.
96-
# RULE 2's input. The script judges it but never fetches it: the judging
97-
# path stays HTTP-free, and the gather is a step of its own so that a
98-
# network failure reads as a failed gather rather than as a verdict about
99-
# somebody's PR.
100-
#
101-
# The endpoint is chosen over `git log base..head` deliberately. It
102-
# returns exactly the set GitHub will squash. The git walk needs the merge
103-
# base present to exclude what is already on the default branch, and the
104-
# checkout above is depth 1 — so on a branch that has merged `main` back
105-
# in, the walk cannot exclude those commits and would report another
106-
# author's landed trailers as this PR's. Deepening until the merge base
107-
# appears is unbounded, and `fetch-depth: 0` clones the whole repository
108-
# to read a handful of messages.
109-
#
110-
# `--paginate` is load-bearing: without it a PR over one page silently
111-
# loses its later commits, and a rule that read half the commits would
112-
# report the unread half as clean. `--jq` emits one JSON object per line,
113-
# and JSON escapes the newlines inside a commit message, so one row really
114-
# is one line. The messages go to a FILE rather than into the environment:
115-
# they are multi-line attacker-controlled text, and a path is inert where
116-
# a body of prose is not.
117-
#
118-
# No pipeline here, on purpose. A `run:` block executes as `bash -e`
119-
# WITHOUT pipefail, so `gh ... | jq ...` would take jq's exit code and a
120-
# failed gather would reach the script as an empty file. It is a single
121-
# redirect, so a failing `gh` fails the step; and if it ever did produce an
122-
# empty file, the script reads zero rows as a failed gather, not as a PR
123-
# with no commits.
124-
- name: Gather the PR's commit messages
125-
env:
126-
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
127-
PR_NUMBER: ${{ github.event.pull_request.number }}
128-
REPO: ${{ github.repository }}
129-
run: >
130-
gh api --paginate "/repos/$REPO/pulls/$PR_NUMBER/commits"
131-
--jq '.[] | {sha: .sha, message: .commit.message}'
132-
> "$RUNNER_TEMP/pr-commits.jsonl"
133-
134-
- name: A PR body may not close the card it is only part of, and no commit may carry a card trailer
96+
- name: A PR body may not close the card it is only part of
13597
env:
13698
PR_BODY: ${{ github.event.pull_request.body }}
13799
PR_NUMBER: ${{ github.event.pull_request.number }}
138-
PR_COMMITS_FILE: ${{ runner.temp }}/pr-commits.jsonl
139100
run: node scripts/check-partof-closing-keyword.mjs

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@
8787
"check:pm-governed-prose": "node scripts/pm/check-governed-prose.mjs --self-test && node scripts/pm/check-governed-prose.mjs",
8888
"check:publish-smoke-pin": "node scripts/publish-smoke-pack.mjs --self-test",
8989
"check:partof-closing-keyword": "node scripts/check-partof-closing-keyword.mjs --self-test",
90+
"check:commit-card-trailers": "node scripts/check-commit-card-trailers.mjs --self-test",
9091
"check:single-claim-paths": "node scripts/check-single-claim-paths.mjs --self-test",
9192
"check:pnpm-filter-targets": "node scripts/pnpm-filter-targets.mjs --self-test && node scripts/check-pnpm-filter-targets.mjs --self-test && node scripts/check-pnpm-filter-targets.mjs",
9293
"check:turbo-task-graph": "node scripts/check-turbo-task-graph.mjs --self-test && node scripts/check-turbo-task-graph.mjs",

0 commit comments

Comments
 (0)