diff --git a/scripts/check-engine-double-contract.mjs b/scripts/check-engine-double-contract.mjs index 045dcb269e..340dafb78d 100644 --- a/scripts/check-engine-double-contract.mjs +++ b/scripts/check-engine-double-contract.mjs @@ -1111,6 +1111,93 @@ function readBaseline() { return JSON.parse(readFileSync(BASELINE_PATH, 'utf8')); } +// ── The ratchet-remedy authority convention (#8435) ────────────────────────── +// +// Four independent PRs, four authors, four brand-new test files, one shift -- +// all four tripped this gate on a hand-rolled `update` double, and all four +// were told about it for the first time by CI. That half is a DISCOVERY-POINT +// problem this message cannot fix: the author writes the double first and meets +// the requirement only when the gate rejects it. +// +// The half this message CAN fix is which remedy it teaches. The text below +// offers two, and it used to offer them symmetrically -- pin the fake, "Or add +// a MEASURED entry to scripts/engine-double-contract.baseline.json". That +// baseline is SHRINK-ONLY, so the second path is not a fix: it is a ratchet +// weakening, and a maintainer action rather than an author's. All four devs +// took the correct path, but they had been told so out of band; a dev reading +// only this output had nothing to go on. Marking the privileged path is the +// cheap, unconditional half of #8435. +// +// Measured as a FARM-LEVEL shape, not a one-gate nit -- see the twin block in +// check-type-check-coverage.mjs for the other instance this PR fixes, and the +// report/finding for the three it does not +// (check-durability-degradation-log-level.mjs, check-role-word.mjs, +// check-driver-conformance.mjs). check-driver-memory-census.mjs is the +// precedent worth copying: it already refuses the weakening remedy outright. +// +// ⛔ Strengthens ratchet governance; weakens nothing. No threshold moves, no +// baseline entry is added, and the verdicts this gate reaches are unchanged -- +// this edits the diagnostic text only. + +/** Kept identical to the twin gate's token so the convention is greppable. */ +const RATCHET_AUTHORITY_MARKER = '⛔ MAINTAINER-ONLY'; + +/** The baseline as the message spells it (BASELINE_PATH is absolute). */ +const BASELINE_REL = 'scripts/engine-double-contract.baseline.json'; + +/** + * How this gate OFFERS the privileged path. A detector rather than a string + * compare, so the self-test can prove it still reaches its subject: a reworded + * offer that stopped matching would make the convention check below pass + * vacuously on every message. + */ +const RATCHET_EXPANSION_OFFER = new RegExp( + `add a MEASURED entry to\\s+${BASELINE_REL.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}`, +); + +/** + * The convention: a message that hands the author the baseline-expanding path + * must say in the same breath that the path is not theirs. Messages that offer + * no such path are unaffected -- RECONCILED tells the author to DELETE or lower + * an entry, which is the ratchet tightening and squarely the author's job. + * + * @param {string} message + * @returns {boolean} + */ +function ratchetRemedyCarriesAuthority(message) { + if (!RATCHET_EXPANSION_OFFER.test(message)) return true; + return message.includes(RATCHET_AUTHORITY_MARKER); +} + +/** + * PINNED's text, named and pure so the self-test can assert on the exact string + * the author reads. Extracted from the audit loop by #8435 for that reason -- + * a message built inline is a message no assertion can reach. + * + * @param {{verb: string, symbols: Set, producer: string, pinCall: string}} slice + * @param {string} file + * @param {Array<{line: number}>} unguarded + * @returns {string} + */ +function pinnedMessage(slice, file, unguarded) { + return ( + `PINNED [${slice.verb}]: ${file} declares ${unguarded.length} engine double(s) whose ` + + `${slice.verb}() does not route through ${[...slice.symbols][0]} ` + + `(line${unguarded.length > 1 ? 's' : ''} ${unguarded.map((d) => d.line).join(', ')}). ` + + `A fake looser than ${slice.producer} is how #4434 shipped a dead REST route with its ` + + `suite green. Open the fake's ${slice.verb} with \`${slice.pinCall}\` from ` + + "'@objectstack/metadata-core' (where the predicate lives since #5619) or from " + + "'@objectstack/objectql' (which re-exports it) — add whichever you pick as a " + + 'devDependency if the package lacks it, and prefer metadata-core when ' + + '@objectstack/objectql DEPENDS ON the package you are pinning, since that reverse edge ' + + 'is a cycle turbo refuses. That is the fix, and the only one of the two you can take on ' + + `your own. ${RATCHET_AUTHORITY_MARKER}, NOT a co-equal option: add a MEASURED entry to ` + + `${BASELINE_REL} saying why not — with ` + + `"verb": ${JSON.stringify(slice.verb)}. That baseline is shrink-only, so an entry weakens a ` + + 'ratchet and needs a maintainer to agree first — do not take this path to get CI green.' + ); +} + // ── Audit ─────────────────────────────────────────────────────────────────── /** One slice's scan over the whole tree. */ @@ -1180,20 +1267,7 @@ function audit() { continue; } if (!entry) { - errors.push( - `PINNED [${slice.verb}]: ${file} declares ${unguarded.length} engine double(s) whose ` - + `${slice.verb}() does not route through ${[...slice.symbols][0]} ` - + `(line${unguarded.length > 1 ? 's' : ''} ${unguarded.map((d) => d.line).join(', ')}). ` - + `A fake looser than ${slice.producer} is how #4434 shipped a dead REST route with its ` - + `suite green. Open the fake's ${slice.verb} with \`${slice.pinCall}\` from ` - + "'@objectstack/metadata-core' (where the predicate lives since #5619) or from " - + "'@objectstack/objectql' (which re-exports it) — add whichever you pick as a " - + 'devDependency if the package lacks it, and prefer metadata-core when ' - + '@objectstack/objectql DEPENDS ON the package you are pinning, since that reverse edge ' - + 'is a cycle turbo refuses. Or add a MEASURED entry to ' - + 'scripts/engine-double-contract.baseline.json saying why not — with ' - + `"verb": ${JSON.stringify(slice.verb)}.`, - ); + errors.push(pinnedMessage(slice, file, unguarded)); continue; } if (unguarded.length > entry.unguarded) { @@ -1970,6 +2044,33 @@ class Svc { seamFiles.some((f) => f.file === 'packages/metadata-protocol/src/protocol.ts' && f.seams.some((x) => x.fn === 'updateData') && f.seams.some((x) => x.fn === 'deleteData'))); + // ── The ratchet-remedy authority convention (#8435) ──────────────────────── + // + // Three assertions, deliberately non-overlapping, so each way this can rot is + // caught by exactly one NAMED failure: (1) the detector still reaches its + // subject, (2) the real message carries the marker, (3) an unmarked offer is + // REJECTED. (3) is what makes (2) worth having -- a predicate that approved + // everything would keep (2) green with the convention gone. Its fixture is + // SYNTHETIC rather than the real message with the marker stripped: derived, + // it also fired on a rewording and misdescribed the cause. + const pinned = pinnedMessage( + { verb: 'update', symbols: new Set(['assertEngineUpdateDispatch']), + producer: 'ObjectQL.update', pinCall: 'assertEngineUpdateDispatch(data, options)' }, + 'packages/plugins/plugin-auth/src/a.test.ts', + [{ line: 72 }], + ); + expect('#8435 — the ratchet-offer DETECTOR still matches PINNED (else the check below is vacuous)', + RATCHET_EXPANSION_OFFER.test(pinned)); + expect(`#8435 — PINNED marks the baseline path ${RATCHET_AUTHORITY_MARKER} (it is shrink-only, so ` + + 'adding an entry is a maintainer action, not the author\'s second option)', + ratchetRemedyCarriesAuthority(pinned)); + const unmarkedOffer = `PINNED: add a MEASURED entry to ${BASELINE_REL} saying why not.`; + expect('#8435 — the synthetic unmarked-offer fixture is still recognised as an offer', + RATCHET_EXPANSION_OFFER.test(unmarkedOffer)); + expect('#8435 — ratchetRemedyCarriesAuthority() REJECTS an offer carrying no marker (proves the ' + + 'predicate discriminates rather than approving everything)', + !ratchetRemedyCarriesAuthority(unmarkedOffer)); + if (failures.length) { for (const f of failures) console.error(` x self-test: ${f}`); console.error(`\ncheck-engine-double-contract --self-test: ${failures.length} failure(s).\n`); diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index 3bcc81ba06..24225d1167 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -1894,6 +1894,68 @@ function measureLedgers(packages, rootName, state) { return measurements; } +// ── The ratchet-remedy authority convention (#8435) ────────────────────────── +// +// A gate that offers two remedies teaches whichever one the author can act on. +// This gate's second remedy -- raise the TEST_DEBT entry -- edits a SHRINK-ONLY +// ledger, so taking it is not a fix at all: it is a ratchet weakening, and +// #8225 had already paid to press plugin-auth's entry from 131 to 111 hours +// before a +1 drift arrived on that same entry the same shift. Raising it would +// have handed part of that cost back with every gate green and nothing anywhere +// recording the reversion. +// +// The ledger's shrink-only semantics were ALREADY written into the message +// below ("frozen debt, not a permission slip"). What was missing is the half a +// reader needs in order to act: WHOSE call it is. An author reading this output +// sees two paths that both turn CI green, and no marker saying one of them is +// not theirs to take. So the fix is not more explanation -- it is an authority +// label on the privileged path. +// +// Measured as a FARM-LEVEL shape, not a one-gate nit: the same structure -- +// second remedy edits a shrink-only ratchet, presented co-equally -- also lives +// in check-engine-double-contract.mjs (which this PR fixes), and in +// check-durability-degradation-log-level.mjs, check-role-word.mjs and +// check-driver-conformance.mjs (which it does not; see the report/finding). +// `check-driver-memory-census.mjs` is the precedent worth copying: its output +// already refuses the weakening remedy outright ("Do NOT add an entry to make +// ..."). +// +// ⛔ This convention STRENGTHENS ratchet governance. Nothing here relaxes a +// threshold, adds a baseline entry, or raises a ledger number -- the verdicts +// this gate reaches are byte-for-byte the ones it reached before. + +/** + * The marker token every gate in the farm uses for the same purpose, kept short + * and identical across gates so it is greppable and reads as one convention + * rather than one author's phrasing. + */ +const RATCHET_AUTHORITY_MARKER = '⛔ MAINTAINER-ONLY'; + +/** + * How this gate OFFERS the privileged path, as a detector rather than a string + * compare. Built from `SELF` so a rename of this file moves both halves + * together; the phrase in front of it is what the self-test pins, because a + * reworded offer that no longer matches would make the convention check below + * pass vacuously on every message. + */ +const RATCHET_EXPANSION_OFFER = new RegExp( + `raise the entry in\\s+${SELF.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}`, +); + +/** + * The convention itself, as a predicate: a message that hands the author the + * ratchet-raising path must say, in the same breath, that the path is not + * theirs. A message that offers no such path is unaffected -- this is an + * authority label, not a vocabulary ban. + * + * @param {string} message + * @returns {boolean} + */ +function ratchetRemedyCarriesAuthority(message) { + if (!RATCHET_EXPANSION_OFFER.test(message)) return true; + return message.includes(RATCHET_AUTHORITY_MARKER); +} + /** * MEASURED's verdict, pure over already-taken measurements so the self-test * pins the semantics without running a compiler. @@ -1922,11 +1984,15 @@ function evaluateMeasurements(measurements) { problems.push( `${m.name}: ${m.ledger} records ${m.recorded} raw tsc error(s), \`tsc --noEmit\` now reports ` + `${m.actual} (+${m.actual - m.recorded}). ${m.ledger} is frozen debt, not a permission slip -- ` + - `the ledger is a ratchet and may only shrink (${REMEASURE_ISSUE}). Fix the new errors, or, if they ` + - `are genuinely irreducible today, raise the entry in ${SELF} AND rewrite its \`note\` to match what ` + + `the ledger is a ratchet and may only shrink (${REMEASURE_ISSUE}). Fix the new errors -- that is ` + + `the author's remedy, and the only one of the two below that you can take on your own. ` + + `${RATCHET_AUTHORITY_MARKER}, NOT a co-equal option: if they are genuinely irreducible today, ` + + `raise the entry in ${SELF} AND rewrite its \`note\` to match what ` + `the pile is now made of: the composition drifts too, and a note still naming only the old errors ` + `reads as "nearly graduated" to the next author while something else entirely has moved in. ` + - `If the delta cannot be attributed, say that in the note rather than inventing composition.`, + `If the delta cannot be attributed, say that in the note rather than inventing composition. ` + + `Raising the entry weakens a shrink-only ratchet and hands back what an earlier PR paid to press ` + + `it down, so it needs a maintainer's agreement first -- do not take this path to get CI green.`, ); } else if (m.actual === 0 && m.recorded > 0) { notes.push( @@ -2535,6 +2601,64 @@ function selfTest() { } } + // ── The ratchet-remedy authority convention (#8435) ──────────────────────── + // + // Three assertions, deliberately non-overlapping, so each way this can rot is + // caught by exactly one NAMED failure: + // + // (1) the detector still reaches its subject -- the only one that fails if + // the offer is reworded out from under `RATCHET_EXPANSION_OFFER`, + // which would make (3) pass vacuously forever after; + // (2) the real emitted message carries the marker -- the only one that + // fails if the label is dropped from the drift text; + // (3) an offer WITHOUT the marker is REJECTED -- the only one that fails if + // the predicate stops discriminating (e.g. is reduced to `return true`). + // + // (3) is what makes (2) worth having: without it, a predicate that approves + // everything would keep this block green while the convention is gone. + const driftMessage = evaluateMeasurements([ + { ledger: 'TEST_DEBT', name: 'plugin-auth', recorded: 111, actual: 112 }, + ]).problems[0]; + + if (!RATCHET_EXPANSION_OFFER.test(driftMessage)) { + failures.push( + '#8435 convention — the ratchet-offer DETECTOR no longer matches the drift message it is ' + + 'written against. Either the offer was reworded (re-point RATCHET_EXPANSION_OFFER at the new ' + + 'wording) or the ratchet path was removed (delete the convention block). Until then the ' + + 'convention check passes vacuously on every message.', + ); + } + if (!ratchetRemedyCarriesAuthority(driftMessage)) { + failures.push( + '#8435 convention — the drift message offers the ratchet-raising path in ' + + `${SELF} without the ${RATCHET_AUTHORITY_MARKER} marker. The ledger is shrink-only, so that ` + + 'path is a maintainer action; presenting it unmarked next to the real fix is what let a +1 ' + + 'drift read as "two ways to go green".', + ); + } + { + // (3)'s fixture is SYNTHETIC, not the real message with the marker stripped + // out. Derived from the real message, this assertion also fired whenever the + // offer was reworded -- two named failures for one rot, and the second one + // misdescribed the cause ("the predicate is not discriminating" when in fact + // the detector had simply stopped matching). Built here from the same + // constant the detector is, it stays green under a rewording, so (1) owns + // that failure alone. Measured, not assumed: this exact case is why. + const unmarkedOffer = `TEST_DEBT drifted upward. raise the entry in ${SELF} AND rewrite its note.`; + if (!RATCHET_EXPANSION_OFFER.test(unmarkedOffer)) { + failures.push( + '#8435 convention — the synthetic unmarked-offer fixture is no longer recognised as an offer, ' + + 'so it cannot test discrimination at all. Re-spell it to match RATCHET_EXPANSION_OFFER.', + ); + } else if (ratchetRemedyCarriesAuthority(unmarkedOffer)) { + failures.push( + '#8435 convention — ratchetRemedyCarriesAuthority() ACCEPTED a message that offers the ' + + 'ratchet-raising path with no marker at all. The predicate is not discriminating, so the ' + + 'assertion above proves nothing.', + ); + } + } + // The counter is the other half that can be silently wrong: over-count and // main goes red for nothing, under-count and the ratchet hands out free // headroom. Multi-line elaborations are the trap -- one TS2322 can print five