From 3618f7e97149bed9c5c3aeaed3fed6b7978d3af9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 20:07:54 +0000 Subject: [PATCH 1/2] fix(devx): teach the os-regen pre-commit hook the deferred merge (#8047) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two in-repo authorities contradicted each other on main. `scripts/pm/os-regen-merge.sh` — the sanctioned landing sequence for a branch touching `merge=os-regen` paths — commits the merge BEFORE regenerating, deliberately: the driver exits 0 while silently dropping one side, so only a separate regeneration commit on a known-good base lets a reviewer read "what main brought" apart from "what the change produces". The `os-regen` pre-commit hook refused exactly that commit. The way out people learned (measured on PR #7851) was to skip the entire pre-commit hook, which trades one false positive for a blanket bypass. Maintainer ruling 2026-08-12: the hook moves. The merge commit is now DEFERRED, not passed: `pre-commit` records `deferred-at ` in the existing marker and lets the commit through. Two properties keep that a split rather than an escape hatch. One commit deep, by construction. A deferral is entered only while MERGE_HEAD exists, a second merge attempted while one is outstanding is refused, and every non-merge commit after it is refused by the ordinary staleness check — so nothing can land between the merge and its discharge. "The immediately following commit" is enforced by there being no other commit it could be. Collected where collection becomes possible. At the moment the merge commit is created, the commit that discharges it does not exist, so pre-commit can only record. The two events that can follow are the next commit (this same check) and the push, so `.githooks/pre-push` — new, registered by the existing `core.hooksPath=.githooks`, 77ms and a marker stat on every push that did not defer — refuses a push that still owes one. An undischarged deferral cannot leave the machine. Prose reconciled across all three authorities that disagreed: the script header (the authority), AGENTS.md §11, and what the hook prints. The hook's refusal now names `scripts/pm/os-regen-merge.sh` instead of advertising a bypass. Tests: `--self-test` grows a decision table over all five states plus a fixture that builds throwaway git repos and replays the two-commit sequence — behaviour about a PAIR of commits, which no single-tree assertion can express. Gates are redirected at the fixture's own package.json so the replay costs no spec build; a mistake there fails safe (a directory without those scripts reads as stale). The pre-push hook's index mode is asserted too — git ignores a non-executable hook in silence. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Q983j8EGKjUs8r14n76TeH --- .githooks/pre-commit | 12 + .githooks/pre-push | 24 ++ AGENTS.md | 11 + scripts/check-regen-pending.mjs | 384 ++++++++++++++++++++++++++++++-- scripts/pm/os-regen-merge.sh | 12 + 5 files changed, 421 insertions(+), 22 deletions(-) create mode 100755 .githooks/pre-push diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 172d788fe4..c07276dd0f 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -5,6 +5,18 @@ # which `pnpm install` runs. Cheap by construction: with no pending marker it # exits before doing any work, which is every commit that did not just merge a # generator-owned artifact (#4675). +# +# One commit is exempt, and only one: the MERGE commit itself, which the +# sanctioned landing sequence (`scripts/pm/os-regen-merge.sh`, step 3) commits +# BEFORE regenerating, so that "what main brought" stays readable apart from +# "what the change produces". That commit records a deferral instead of being +# refused; the immediately following commit must discharge it, and +# `.githooks/pre-push` refuses the push if none ever does (#8047). +# +# ⚠️ git does not run this hook for a merge it completes ITSELF — only for one +# you finish with `git commit`. A clean auto-committed merge therefore lands +# with the marker untouched and the refusal falls on the next commit, which is +# the same collection point either way. if [ -z "$OS_SKIP_REGEN_CHECK" ]; then node "$(git rev-parse --show-toplevel)/scripts/check-regen-pending.mjs" || exit 1 diff --git a/.githooks/pre-push b/.githooks/pre-push new file mode 100755 index 0000000000..126341db8e --- /dev/null +++ b/.githooks/pre-push @@ -0,0 +1,24 @@ +#!/bin/sh +# Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +# +# The collection half of the os-regen deferred merge (#8047). `pre-commit` may +# accept a MERGE commit whose regeneration is deferred to the next commit — but +# at the moment it decides, that next commit does not exist yet, so it can only +# RECORD the deferral. Something has to be the event that notices the deferral +# was never discharged. Only two events can follow a merge commit: another +# commit (which `pre-commit` already refuses while the artifacts are stale) and +# the push. This is the push. +# +# Registered by the same `core.hooksPath=.githooks` that registers `pre-commit`, +# so it needs no change to `scripts/setup-git-hooks.mjs` and no separate opt-in. +# +# Cheap by construction, exactly like `pre-commit`: with no pending marker the +# script exits before doing any work, which is every push in this repo that did +# not just defer a merge. The ref list git writes on stdin is drained and +# ignored on purpose — the marker is per-worktree state, not per-ref state, so +# which refs are being pushed cannot change the answer. + +if [ -z "$OS_SKIP_REGEN_CHECK" ]; then + cat >/dev/null + node "$(git rev-parse --show-toplevel)/scripts/check-regen-pending.mjs" --pre-push || exit 1 +fi diff --git a/AGENTS.md b/AGENTS.md index d47fad31f2..e6efc6b497 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -364,6 +364,17 @@ Even inside your own worktree, operate defensively: `$GIT_DIR/os-regen-pending` and `pre-commit` refuses the commit until those artifacts check clean. Sequence after a merge unchanged from §9: rebuild, then `check:generated --fix` — you just cannot forget it. Worth knowing: + - **The MERGE commit itself is the one exemption, and it is a deferral, not a + pass** (#8047). `scripts/pm/os-regen-merge.sh` is the in-repo authority for + landing one of these branches, and its step 3 commits the merge **before** + regenerating on purpose: the driver exits 0 while silently dropping one side, so + only a separate regeneration commit on a known-good base lets a reviewer read + "what main brought" apart from "what the change produces". `pre-commit` records + that merge as a deferral and then holds you to it — the immediately following + commit must discharge it (every commit until then is refused, and a second merge + cannot defer on top of an outstanding one), and `.githooks/pre-push` refuses a + push that still owes one. ⛔ So this step never needs `--no-verify`, which was + the old spelling and skips *every* pre-commit check rather than this one. - **The driver is a LOCAL facility** — the merge queue rebuilds server-side where no custom driver runs, so the three hottest artifacts are **sharded** per category/entry (`authorable-surface/`, `json-schema.manifest/`, `api-surface/`) to diff --git a/scripts/check-regen-pending.mjs b/scripts/check-regen-pending.mjs index ddc0be61c9..5598c98742 100755 --- a/scripts/check-regen-pending.mjs +++ b/scripts/check-regen-pending.mjs @@ -19,6 +19,37 @@ * whether you regenerated them or the merge simply did not change them, the * marker is removed and the commit proceeds. * + * ## The deferred merge (#8047) + * + * One commit is exempt from the refusal above, and only one: the MERGE commit + * itself. `scripts/pm/os-regen-merge.sh` — the sanctioned landing sequence, and + * the in-repo authority for it — takes main's side of every generated path and + * **commits the merge before regenerating anything**, deliberately, because the + * driver exits 0 while silently dropping one side: a clean merge and a lost + * baseline are indistinguishable, and only a separate regeneration commit on a + * known-good base lets a reviewer read "what main brought" apart from "what my + * change produces". Refusing that commit made one in-repo authority forbid what + * another in-repo tool deliberately produces, and the way out people learned was + * to skip the ENTIRE pre-commit hook — trading one false positive for a blanket + * bypass. Measured on PR #7851, ruled 2026-08-12. + * + * So a merge commit whose artifacts are stale is **deferred, not passed**: the + * deferral is recorded in the marker (`deferred-at `) and the + * commit proceeds. Two properties make that a split rather than an escape hatch: + * + * - **It is one commit deep, by construction.** A deferral is only ever entered + * while `MERGE_HEAD` exists, and a second merge attempted while one is still + * outstanding is refused. Every non-merge commit after it is refused too — by + * the ordinary staleness check, unchanged — so nothing can land between the + * merge and its discharge. "The immediately following commit" is enforced by + * there being no other commit it could be. + * - **The discharge is checked where it becomes knowable.** At the instant the + * merge commit is created, the commit that discharges it does not exist yet, + * so `pre-commit` can only RECORD the deferral. The two events that can + * follow are the next commit (this same check, which refuses while stale) and + * the push (`.githooks/pre-push`, which runs this with `--pre-push`). A + * deferral that is never discharged therefore cannot leave the machine. + * * ## The dist trap * * `gen:api-surface` reads the BUILT `dist/*.d.ts`. On a stale dist it does not @@ -29,11 +60,13 @@ * * Usage: * node scripts/check-regen-pending.mjs # pre-commit - * node scripts/check-regen-pending.mjs --self-test # no repo state touched + * node scripts/check-regen-pending.mjs --pre-push # pre-push: never defers + * node scripts/check-regen-pending.mjs --self-test # fixtures only, no repo state */ -import { execFileSync, execSync } from 'node:child_process'; -import { existsSync, readFileSync, readdirSync, rmSync, statSync } from 'node:fs'; +import { execFileSync, execSync, spawnSync } from 'node:child_process'; +import { appendFileSync, existsSync, mkdtempSync, readFileSync, readdirSync, rmSync, statSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -42,6 +75,22 @@ import { PENDING_MARKER, entryForPath } from './regen-artifacts.mjs'; const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); const SPEC_DIR = join(REPO_ROOT, 'packages/spec'); +/** + * Where the `check:*` gates are spawned. `--self-test`'s fixtures point this at a + * throwaway package so the two-commit sequence can be replayed without spawning + * the real spec gates; nothing else sets it. A mistake here fails SAFE — a + * directory without those scripts makes pnpm exit non-zero, which reads as stale. + */ +const GATE_CWD = process.env.OS_REGEN_GATE_CWD || SPEC_DIR; + +/** + * Marker line recording a deferral, distinguished from the driver's path lines by + * a prefix no repo path can have (it contains a space; `%P` pathnames do not). + * An unrecognised line would be reported as an unknown pending path — blocked, + * the conservative direction — rather than silently ignored. + */ +const DEFERRAL_PREFIX = 'deferred-at '; + /** Newest mtime under `dir` for files matching `pred`, or 0 when there are none. */ function newestMtime(dir, pred, depth = 0) { if (depth > 12 || !existsSync(dir)) return 0; @@ -95,36 +144,98 @@ export function schemaTreeIsStale(specDir = SPEC_DIR) { return newestMtime(join(specDir, 'src'), (n) => n.endsWith('.ts') && !n.endsWith('.test.ts')) > tree; } -function markerPath() { - const gitDir = execFileSync('git', ['rev-parse', '--absolute-git-dir'], { encoding: 'utf8' }).trim(); - return join(gitDir, PENDING_MARKER); +/** + * This worktree's git dir — `--absolute-git-dir` resolves to `.git/worktrees/` + * in a linked worktree, so the marker and its deferral are per-worktree state and + * two agents merging in parallel cannot collect each other's debt. + */ +function gitDirPath() { + return execFileSync('git', ['rev-parse', '--absolute-git-dir'], { encoding: 'utf8' }).trim(); } -function readPending(marker) { - if (!existsSync(marker)) return []; - return [...new Set(readFileSync(marker, 'utf8').split('\n').map((l) => l.trim()).filter(Boolean))]; +/** The marker's two kinds of line: the driver's deferred paths, and our deferral record. */ +export function readMarker(marker) { + if (!existsSync(marker)) return { paths: [], deferral: null }; + const lines = readFileSync(marker, 'utf8').split('\n').map((l) => l.trim()).filter(Boolean); + const record = lines.find((l) => l.startsWith(DEFERRAL_PREFIX)); + const [head, ...mergeHeads] = record ? record.slice(DEFERRAL_PREFIX.length).trim().split(/\s+/) : []; + return { + paths: [...new Set(lines.filter((l) => !l.startsWith(DEFERRAL_PREFIX)))], + deferral: record ? { head, mergeHeads } : null, + }; +} + +/** Append the deferral record. Appended, never rewritten — the driver owns the path lines. */ +export function recordDeferral(marker, { head, mergeHeads }) { + appendFileSync(marker, `${DEFERRAL_PREFIX}${[head, ...mergeHeads].join(' ')}\n`); +} + +/** + * Is a merge commit being created right now? `MERGE_HEAD` exists only between a + * stopped merge and the commit that completes it, which is exactly the window + * `pre-commit` runs in for a merge commit. + * + * ⚠️ A merge that completes WITHOUT stopping never reaches this file: git does not + * run `pre-commit` for the commit it makes itself (verified, git 2.43). Such a + * merge lands with the marker set and untouched, and the refusal falls on the next + * commit — which is the deferral's discharge point either way. + */ +export function mergeInProgress(gitDir) { + const file = join(gitDir, 'MERGE_HEAD'); + if (!existsSync(file)) return null; + const mergeHeads = readFileSync(file, 'utf8').split('\n').map((l) => l.trim()).filter(Boolean); + let head = ''; + try { + head = execFileSync('git', ['rev-parse', 'HEAD'], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }).trim(); + } catch { + head = '(root)'; // A merge as the first commit has no HEAD. Vanishingly rare, not an error. + } + return { head, mergeHeads }; +} + +/** + * The whole state machine, as a pure function — five cases, no I/O, so the + * self-test can pin every one of them including the ones a fixture cannot reach. + * + * `blocked` is how many artifacts failed their gate; `merging` is `MERGE_HEAD` + * present; `deferral` is a record already outstanding; `allowDefer` is false on + * `--pre-push`, where deferring would defeat the point of checking at all. + */ +export function decide({ blocked, merging, deferral, allowDefer = true }) { + if (!blocked) return deferral ? 'discharged' : 'clear'; + if (merging && deferral) return 'refuse-second-deferral'; + if (merging && allowDefer) return 'defer'; + if (deferral) return 'refuse-undischarged'; + return 'refuse-stale'; } function runCheck(script) { try { - execSync(`pnpm -s ${script}`, { cwd: SPEC_DIR, stdio: ['ignore', 'pipe', 'pipe'] }); + execSync(`pnpm -s ${script}`, { cwd: GATE_CWD, stdio: ['ignore', 'pipe', 'pipe'] }); return { ok: true, output: '' }; } catch (err) { return { ok: false, output: `${err?.stdout?.toString() ?? ''}${err?.stderr?.toString() ?? ''}`.trim() }; } } -function main() { - const marker = markerPath(); - const pending = readPending(marker); - if (!pending.length) return 0; +function main({ prePush = false } = {}) { + const gitDir = gitDirPath(); + const marker = join(gitDir, PENDING_MARKER); + const { paths: pending, deferral } = readMarker(marker); + if (!pending.length) { + // Nothing deferred (or a record with no paths left to owe): the marker has no + // debt to collect, so it cannot get stuck. Same clearing property as before. + if (deferral) rmSync(marker, { force: true }); + return 0; + } + const merging = prePush ? null : mergeInProgress(gitDir); const entries = pending.map((p) => ({ path: p, entry: entryForPath(p) })).filter((x) => x.entry); const unknown = pending.filter((p) => !entryForPath(p)); console.error( `\nos-regen: ${pending.length} generated artifact(s) were merged WITHOUT a text merge and must be ` - + `regenerated from the merged tree before this commit.\n`, + + `regenerated from the merged tree${prePush ? ' before this push' : ' before this commit'}.\n`, ); // Group by gate: `gen:schema` owns two artifacts, so running it twice is waste. @@ -177,18 +288,70 @@ function main() { console.error(` ✗ ${p} — recorded as pending but absent from scripts/regen-artifacts.mjs (cannot verify)`); } - if (blocked) { + const action = decide({ blocked, merging, deferral, allowDefer: !prePush }); + const owed = `the ${blocked} stale artifact(s) above`; + + if (action === 'defer') { + recordDeferral(marker, merging); console.error( - `\nRegenerate the ${blocked} stale artifact(s) above, \`git add\` them, and commit again.\n` + `\nos-regen: this is a MERGE commit — ${owed} are DEFERRED to the next commit, not passed.\n` + + ` Recorded against ${merging.head.slice(0, 12)} + ${merging.mergeHeads.map((h) => h.slice(0, 12)).join(' ')}.\n` + + ' The merge lands as its own commit on purpose (`scripts/pm/os-regen-merge.sh` step 3): the\n' + + ' os-regen driver exits 0 while silently dropping one side, so only a separate regeneration\n' + + ' commit on a known-good base tells "what main brought" apart from "what your change produces".\n' + + '\n ⚠️ The NEXT commit must discharge this — regenerate, `git add`, commit. Until it does, every\n' + + ' commit is refused, a second merge cannot defer on top, and `git push` is refused too.\n', + ); + return 0; + } + + if (action === 'refuse-second-deferral') { + console.error( + `\nos-regen: a deferral is already outstanding (taken at ${deferral.head.slice(0, 12)}) and ${owed}\n` + + ' are still stale, so THIS merge cannot defer on top of it. A deferral is one commit deep by\n' + + ' construction — any deeper and it is an escape hatch, not the split-commit procedure.\n' + + ' Discharge the first one (regenerate + commit), then merge again.\n', + ); + return 1; + } + + if (action === 'refuse-undischarged') { + console.error( + `\nos-regen: the deferral taken on the merge commit at ${deferral.head.slice(0, 12)} is UNDISCHARGED —\n` + + (prePush + ? ` this push still owes ${owed}. A merge may defer its regeneration to the next commit;\n` + + ' it may not defer it past the push, which is the last moment anything local can see it.\n' + : ` this is the commit that owes ${owed}. Regenerate them and \`git add\` them.\n`) + + ' (`scripts/pm/os-regen-merge.sh` prints the step-4 chain for the surface you touched.)\n', + ); + return 1; + } + + if (action === 'refuse-stale') { + console.error( + `\nRegenerate ${owed}, \`git add\` them, and ${prePush ? 'commit the result before pushing' : 'commit again'}.\n` + ' This check clears itself the moment they are current — nothing to reset by hand.\n' - + ' Bypass with --no-verify only if you intend CI to catch it: every one of these has a\n' - + ' required gate on the PR.\n', + + ' Landing a merge? `bash scripts/pm/os-regen-merge.sh` runs the sanctioned sequence — it commits\n' + + ' the merge first (this hook records that as a deferral) and regeneration follows as its own\n' + + ' commit. Every artifact above also has a required gate on the PR.\n', ); return 1; } + if (prePush && blocked) { + // Unreachable via `decide` — `allowDefer: false` routes every blocked push into + // one of the refusals above. Kept as a floor: a future case that forgets the + // push path must not let a stale artifact out of the machine by falling through. + console.error('\nos-regen: refusing to push with deferred artifacts still stale.\n'); + return 1; + } + rmSync(marker, { force: true }); - console.error('os-regen: all deferred artifacts are current — marker cleared.\n'); + console.error( + action === 'discharged' + ? 'os-regen: deferred regeneration discharged — all artifacts current, marker cleared.\n' + : 'os-regen: all deferred artifacts are current — marker cleared.\n', + ); return 0; } @@ -196,6 +359,177 @@ function main() { // import — only when this file IS the entry point. const invokedDirectly = process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url); +/** + * Replay the deferred-merge sequence against a THROWAWAY git repo (#8047). + * + * This behaviour is a property of a PAIR of commits — accept here, collect there + * — which no single-tree assertion can express, so the fixture builds the pair. + * The worked precedent it is shaped after is PR #7851: merge commit `cbea40d` + * (parents `c57e636` + `e3c8ed0`) followed by regeneration `7e4799f`. + * + * The gates are redirected at the fixture's own `package.json` via + * `OS_REGEN_GATE_CWD`, so `check:spec-changes` is a one-line stub the fixture + * flips from failing to passing — the two-commit shape is what is under test, not + * the spec gates, and spawning the real ones here would make the self-test cost + * a full spec build. + */ +function fixtureSelfTest() { + const dir = mkdtempSync(join(tmpdir(), 'os-regen-defer-')); + const git = (args, opts = {}) => + execFileSync('git', args, { cwd: dir, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], ...opts }); + const results = []; + const check = (label, cond) => { + results.push(cond); + console.log(` ${cond ? '✓' : '✗'} ${label}`); + }; + + /** Run the real script inside the fixture, with the stub gate in the given state. */ + const runHook = (gate, args = []) => { + writeFileSync( + join(dir, 'package.json'), + `${JSON.stringify({ name: 'os-regen-fixture', scripts: { 'check:spec-changes': gate === 'clean' ? 'exit 0' : 'exit 1' } }, null, 2)}\n`, + ); + // `spawnSync`, not `execFileSync`: every message this script prints goes to + // STDERR, which execFileSync returns only on the failure path — capturing the + // accept-path wording is half of what is under test here. + const r = spawnSync(process.execPath, [fileURLToPath(import.meta.url), ...args], { + cwd: dir, + encoding: 'utf8', + env: { ...process.env, OS_REGEN_GATE_CWD: dir }, + }); + return { code: r.status ?? 1, out: `${r.stdout ?? ''}${r.stderr ?? ''}` }; + }; + + try { + git(['init', '-q', '-b', 'main', '.']); + git(['config', 'user.email', 'fixture@objectstack.test']); + git(['config', 'user.name', 'os-regen fixture']); + git(['config', 'core.hooksPath', '/dev/null']); // the fixture drives the script itself + const gitDir = git(['rev-parse', '--absolute-git-dir']).trim(); + const marker = join(gitDir, PENDING_MARKER); + const pendingPath = 'packages/spec/spec-changes.json'; // a real REGEN_ARTIFACTS entry + const write = (f, c) => writeFileSync(join(dir, f), c); + + write('f.txt', 'base\n'); + git(['add', '-A']); + git(['commit', '-qm', 'base']); + git(['checkout', '-qb', 'side']); + write('s.txt', 'side\n'); + git(['add', '-A']); + git(['commit', '-qm', 'side']); + git(['checkout', '-q', 'main']); + write('f.txt', 'main moved\n'); + git(['add', '-A']); + git(['commit', '-qm', 'main moves']); + + // A merge left uncommitted — the state `pre-commit` sees when it runs for a + // merge commit — with the driver's marker set, artifacts stale. + git(['merge', '--no-commit', '--no-ff', 'side'], { stdio: ['ignore', 'pipe', 'pipe'] }); + writeFileSync(marker, `${pendingPath}\n`); + + const merge = runHook('stale'); + check('a MERGE commit with stale artifacts is ACCEPTED as a deferral', merge.code === 0); + check(' …and says so — "DEFERRED", not a silent pass', /DEFERRED to the next commit/.test(merge.out)); + check(' …and points at the sanctioned procedure, never at skipping the hook', + /os-regen-merge\.sh/.test(merge.out) && !/no-verify/.test(merge.out)); + const recorded = readMarker(marker).deferral; + check(' …recording the merge it was taken on', Boolean(recorded?.head) && recorded.mergeHeads.length === 1); + git(['commit', '-q', '--no-verify', '-m', 'merge side (regeneration follows)']); + + // Still stale one commit later: the deferral is now due, and this is where the + // pre-fix hook and this one must AGREE — both refuse. + const due = runHook('stale'); + check('the NEXT commit is REFUSED while the deferral is undischarged', due.code === 1); + check(' …naming the merge that owes it', /UNDISCHARGED/.test(due.out)); + + // A second merge cannot stack a second deferral on the first. + git(['checkout', '-qb', 'side2', 'main~1']); + write('s2.txt', 'side2\n'); + git(['add', '-A']); + git(['commit', '-qm', 'side2']); + git(['checkout', '-q', 'main']); + git(['merge', '--no-commit', '--no-ff', 'side2'], { stdio: ['ignore', 'pipe', 'pipe'] }); + const second = runHook('stale'); + check('a SECOND merge cannot defer on top of an outstanding deferral', second.code === 1); + check(' …so the exemption stays one commit deep', /one commit deep/.test(second.out)); + git(['merge', '--abort']); + + // The push is the other event that can follow a merge: it must not carry an + // undischarged deferral off the machine. + const push = runHook('stale', ['--pre-push']); + check('`--pre-push` REFUSES an undischarged deferral', push.code === 1); + const pushDefers = /DEFERRED to the next commit/.test(push.out); + check(' …and never defers, whatever the merge state', !pushDefers); + + // Discharge: the artifacts come current, exactly as the regeneration commit makes them. + const discharged = runHook('clean'); + check('regeneration DISCHARGES the deferral and clears the marker', discharged.code === 0); + check(' …saying which of the two clearing paths ran', /discharged/.test(discharged.out)); + check(' …and the marker is gone, so nothing can get stuck', !existsSync(marker)); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + return results.every(Boolean); +} + +/** + * The pre-push hook must be executable **in the index**, not just on this disk — + * git silently ignores a non-executable hook, printing at most an + * `advice.ignoredHook` note nobody reads. `git-merge-regen.mjs --self-test` + * already asserts this for `.githooks/pre-commit`; the assertion lives here for + * its sibling because `check:merge-driver` runs both halves, so the coverage + * lands in the same gate either way, and a disarmed pre-push means an + * undischarged deferral leaves the machine in silence. + */ +function prePushIsArmedSelfTest() { + let mode = ''; + try { + mode = execFileSync('git', ['ls-files', '-s', '.githooks/pre-push'], { + cwd: REPO_ROOT, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }).trim().split(/\s+/)[0] ?? ''; + } catch (err) { + console.log(` ✗ could not stat .githooks/pre-push: ${err?.message ?? err}`); + return false; + } + const ok = mode === '100755'; + console.log( + ok + ? ' ✓ .githooks/pre-push is executable in the index (100755)' + : ` ✗ .githooks/pre-push is mode ${mode || ''} in the index, not 100755.\n` + + ' Git IGNORES a non-executable hook — the deferral would never be collected.\n' + + ' Fix: git update-index --chmod=+x .githooks/pre-push', + ); + return ok; +} + +/** The five `decide` cases, including the ones a fixture cannot reach. */ +function decisionTableSelfTest() { + const cases = [ + [{ blocked: 0, merging: null, deferral: null }, 'clear'], + [{ blocked: 0, merging: null, deferral: { head: 'a' } }, 'discharged'], + [{ blocked: 2, merging: { head: 'a' }, deferral: null }, 'defer'], + [{ blocked: 2, merging: { head: 'a' }, deferral: { head: 'b' } }, 'refuse-second-deferral'], + [{ blocked: 2, merging: null, deferral: { head: 'b' } }, 'refuse-undischarged'], + [{ blocked: 2, merging: null, deferral: null }, 'refuse-stale'], + // `--pre-push`: deferring is off, so a merge in progress cannot buy a pass. + [{ blocked: 2, merging: { head: 'a' }, deferral: null, allowDefer: false }, 'refuse-stale'], + [{ blocked: 2, merging: { head: 'a' }, deferral: { head: 'b' }, allowDefer: false }, 'refuse-second-deferral'], + ]; + let ok = true; + for (const [input, expected] of cases) { + const got = decide(input); + if (got !== expected) ok = false; + console.log( + ` ${got === expected ? '✓' : '✗'} blocked=${input.blocked} merging=${input.merging ? 'y' : 'n'} ` + + `deferral=${input.deferral ? 'y' : 'n'} allowDefer=${input.allowDefer !== false ? 'y' : 'n'} → ${got}` + + (got === expected ? '' : ` (expected ${expected})`), + ); + } + return ok; +} + if (invokedDirectly) { if (process.argv.includes('--self-test')) { // Touches no repo state: the interesting logic is the staleness rules, and @@ -204,9 +538,15 @@ if (invokedDirectly) { console.log(`${noDist ? '✓' : '✗'} a directory with no dist/ reads as STALE (conservative default)`); const noTree = schemaTreeIsStale(join(REPO_ROOT, 'scripts')) === true; console.log(`${noTree ? '✓' : '✗'} a directory with no json-schema/ reads as STALE (conservative default)`); - const ok = noDist && noTree; + console.log('\ndeferred-merge collection point:'); + const armed = prePushIsArmedSelfTest(); + console.log('\ndeferred-merge decision table:'); + const table = decisionTableSelfTest(); + console.log('\ndeferred-merge sequence, replayed on a throwaway repo:'); + const fixture = fixtureSelfTest(); + const ok = noDist && noTree && armed && table && fixture; console.log(ok ? '\n✓ check-regen-pending self-test passed.' : '\n✗ self-test failed.'); process.exit(ok ? 0 : 1); } - process.exit(main()); + process.exit(main({ prePush: process.argv.includes('--pre-push') })); } diff --git a/scripts/pm/os-regen-merge.sh b/scripts/pm/os-regen-merge.sh index 50b47248b5..1ae5e78b90 100644 --- a/scripts/pm/os-regen-merge.sh +++ b/scripts/pm/os-regen-merge.sh @@ -29,6 +29,18 @@ # bogus 5xx assertions in the rest package — rerun # `pnpm --filter @objectstack/spec gen:openapi` to restore. # +# ## Step 3 and the pre-commit hook agree (#8047) +# +# They used to contradict each other: this script deliberately produces the +# commit the `os-regen` pre-commit hook refused, so following the procedure +# meant skipping the hook — and `--no-verify` skips EVERY pre-commit check, not +# just this one. Ruled 2026-08-12: the hook moved. It now recognises a merge +# commit whose artifacts are stale and records a DEFERRAL rather than refusing, +# then holds you to it — every commit after the merge is refused until the +# regeneration lands, a second merge cannot defer on top of an outstanding one, +# and `.githooks/pre-push` refuses a push that still owes one. So step 3 needs +# no bypass, and step 4 is what clears the marker. +# # The os-regen path list is read from .gitattributes AT RUN TIME — the one copy # that cannot rot is the one that does not exist. From 6fd9b6df1fd33c3f8e9c322f78ced78aa9af11ea Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 20:13:03 +0000 Subject: [PATCH 2/2] fix(devx): drop the issue-ID citation from the AGENTS.md deferral bullet `check:pm-skill-id-lint` refuses issue numbers in AGENTS.md operative text: a lesson has to be self-contained (failure mode + discipline + boundary) and a ruling keeps its date, so the number carries no information a reader needs. The bullet already states the mechanism in full; the citation becomes the ruling's date. Gate green locally, self-test included. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Q983j8EGKjUs8r14n76TeH --- AGENTS.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e6efc6b497..9e79f13901 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -365,8 +365,9 @@ Even inside your own worktree, operate defensively: artifacts check clean. Sequence after a merge unchanged from §9: rebuild, then `check:generated --fix` — you just cannot forget it. Worth knowing: - **The MERGE commit itself is the one exemption, and it is a deferral, not a - pass** (#8047). `scripts/pm/os-regen-merge.sh` is the in-repo authority for - landing one of these branches, and its step 3 commits the merge **before** + pass** (maintainer ruling 2026-08-12). `scripts/pm/os-regen-merge.sh` is the + in-repo authority for landing one of these branches, and its step 3 commits + the merge **before** regenerating on purpose: the driver exits 0 while silently dropping one side, so only a separate regeneration commit on a known-good base lets a reviewer read "what main brought" apart from "what the change produces". `pre-commit` records