Skip to content

Commit 5d66e38

Browse files
claude[bot]claude
andauthored
fix(ci): refuse card relations in commit messages at push, and re-scope the PR-time gate to the body (#17745)
Part of #17727 The card-trailer rule was enforced on the pull request, over its commit list. By the time it spoke, the cheap repair was gone: a new commit on top JOINS that list, and removing the message is the history rewrite this repository forbids — so two rounds, hours apart, each paid a full redo (a new branch, the diff re-applied, a new PR, the old one closed as superseded). This moves the refusal to the moment the repair is still free, closes the spelling gap that let the forbidden form through, and re-scopes the PR-time gate now that the squash message comes from the body. ## Three parts **1. A pre-push refusal — `scripts/check-commit-card-trailers.mjs`, called from `.githooks/pre-push`.** It judges exactly the commits a push would publish: the local sha, minus the remote sha the push protocol hands it, minus every remote-tracking ref. Everything it judges is therefore UNPUBLISHED, which is what lets it prescribe an ordinary reword instead of a rewrite — the whole trade the move buys. It adds no grammar: the three relation extractors are the half-state sweep's, imported and read at `markdown: false` (a commit message is not markdown, so backticks do not defuse a trailer). A fourth shape is new — a bare `#N` in TRAILER position, where a declaration needs no keyword beside it; a reference in ordinary body prose is deliberately left alone. `.githooks/pre-push` keeps the os-regen deferral refusal it carried, unchanged in behaviour. The ref lines git writes on stdin used to be drained and discarded; they are now read once and handed to the check whose whole input they are. **2. The spelling gap — `partOfRe` and `refsRe` in `scripts/pm/check-half-states.mjs`.** Measured before the change, on the commit surface: | spelling | before | after | |---|---|---| | `Part of #N` | HIT | HIT | | `Part-of #N` | MISS | HIT | | `Part of: #N` | MISS | HIT | | `Part-of: #N` | MISS | HIT | | `Refs #N` | HIT | HIT | | `Refs: #N` | MISS | HIT | | `Fixes: #N` | HIT | HIT | Every dispatch order in this repo lists `Part-of` as a forbidden form, so a dev who greps for it before pushing gets 0 and trusts a net with no thread in that square. The separator is now the one `closingKeywordRe` already ships (an optional colon), and the word takes a hyphen where it takes a space — a strict superset, so nothing that matched before stops matching. `Refs:` is the same colon gap in the sibling relation and is the git-trailer spelling of that declaration; it is fixed in the same edit. `Ref`, `References` and `Parts of` stay out, as before. One pinned case flipped and was replaced rather than reworded: it asserted that `Refs: #N` was *not* the spelling, which pinned the gap rather than a decision. **3. The PR-time gate is re-scoped — `check:partof-closing-keyword`.** Its RULE 2 rested on one premise: the squash message is assembled from the COMMIT MESSAGES. That premise is gone — `squash_merge_commit_message` is `PR_BODY` (paired with `PR_TITLE`, the only combination GitHub accepts it in), so every squash, queue merges included, lands the body. RULE 2 and its commit-list input are removed; the gather step and the `pull-requests: read` scope it needed go with them. RULE 1 and RULE 3 are untouched, the numbering is left alone (the numbers are the order the rules were learned), and the header now says where RULE 2 went and what the move gives up: a hook is registered per clone, so a clone that never ran `pnpm install` pushes unchecked. That is the ruled trade; CI holds the self-test. ## Measured, live Four legs on this branch, against the real hook (`core.hooksPath=.githooks`): - LIT — the two real commits of this PR pushed clean: `check:commit-card-trailers: 2 commit message(s) on this push carry no card relation.` - DARK — a commit carrying `Part of #17727` refused the push, exit 1, naming `commit 1c4b62ca7 ("chore: dark control leg one")`, `line 3`. - DARK — the colon spelling `Part-of: #17727` refused, naming `commit 1bb31a97e`, `line 3`. This is the form that passed before this PR. - DARK — a bare reference in trailer position (`Issue: #17727`) refused, naming `commit d18d3e2d2`, `line 3`, as a trailer declaration rather than as a relation. Each dark leg was an empty commit, removed with `git reset --hard` to the pushed tip afterwards; the branch tip is unchanged by them. ## Self-tests - `check:commit-card-trailers` — 56 cases, 8 batteries with a pinned floor. Includes the three substring shapes that must stay GREEN, each measured against the hand grep this replaces: the stem in ordinary prose (`a closed ten-member enum`), in an identifier (`findClosestMatches`), and in an ordinary word (`fixture`) — a bare-stem grep flags all three, and this must not. - `check:partof-closing-keyword` — 45 cases (from 82: RULE 2's batteries moved out), roster and floor updated in the same edit so the removal cannot read as batteries that stopped running. - `check:pm-half-states` — 3616 cases (from 3606). ## Acceptance notes - The residual this PR does not close: a hook is per-clone and reports to nobody, so enforcement depends on `pnpm install` having run in that clone. Noted, not filed — the ruling chose this trade explicitly, and the alternative (a PR-time read of a commit list whose text no longer lands) is what this PR removes. - `.claude/agents/os-dev.md` gains its one-line pointer in a separate PR: it is a governed surface with its own merge path. --- _Generated by [Claude Code](https://claude.ai/code/session_01MCLBsUgfykL74aU716rzVK)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ec739e6 commit 5d66e38

7 files changed

Lines changed: 1019 additions & 753 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)