diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c0fae7d1e0..5589d0920e 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -76,6 +76,24 @@ jobs: - name: Slot-lookup ratchet run: pnpm check:slot-lookup + # Engine query-options erasure ratchet (#4918). The slot-lookup rule above + # protects the service LOOKUP; this one protects what you pass to the + # service you looked up. `IDataEngine.find/findOne/count/aggregate` declare + # their options as `EngineQueryOptions` & co., and for an internal caller + # `tsc` is the ONLY channel enforcing them — the protocol's ingress + # normalizer never sees a direct engine call, and the options schemas are + # not `.strict()`, so an unknown key is silently DROPPED. #4674 is the + # bill: two queries sorted by `direction` instead of `order`, both with a + # `limit`, so both returned the OLDEST rows — audit history that never + # showed recent changes, and a search that truncated away fresh records. + # #4720 restored those two sites and #4721 closed the external callers; + # this stops the shape regrowing internally. Same ratchet mechanism as + # slot-lookup for the non-test residual, plus one aggregate decrease-only + # number for test code (a test whose subject IS off-contract engine input + # must be able to build it). Runs its own --self-test first. + - name: Engine query-options erasure ratchet + run: pnpm check:query-options-erasure + # Raw control-byte guard (#3127 / #4890 / #5157 / #5460). Scans every # tracked TEXT file for a raw ASCII control byte — 0x00-0x08, 0x0b, 0x0c, # 0x0e-0x1f and 0x7f, i.e. everything except tab/LF/CR. Two distinct diff --git a/eslint.config.mjs b/eslint.config.mjs index d0206dba67..fe0909f885 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -212,6 +212,198 @@ const slotLookupPlugin = { }, }; +// --------------------------------------------------------------------------- +// [#4918] Engine query-options `any`-erasure guard. +// +// The same failure as the slot-lookup rule above, one layer further in: the +// contract exists, `tsc` is willing to enforce it, and one annotation switches +// that off for the call site while looking identical to code that has it. +// +// `IDataEngine.find/findOne/count/aggregate` declare their options as +// `EngineQueryOptions` / `EngineCountOptions` / `EngineAggregateOptions` +// (`packages/spec/src/contracts/data-engine.ts`), and `IDataDriver` declares the +// same slots as `QueryAST` + `DriverOptions`. For an INTERNAL caller `tsc` is +// the ONLY enforced channel on that path: the protocol's ingress normalizer does +// not run on calls the protocol itself makes to `this.engine.find`, and the +// options schemas are not `.strict()`, so an unknown key is silently DROPPED +// rather than rejected. Erase the type and a wrong key becomes a no-op that +// nothing anywhere reports. +// +// #4674 is the bill: two internal queries spelled their sort +// `{ field, direction: 'desc' }` — `IReportService`'s vocabulary — where the +// QueryAST shape is `SortNodeSchema` = `{ field, order }`. Both drivers +// normalize off `.order` with no fallback, so both queries ran ASCENDING, and +// because both carried a `limit` the wrong direction changed WHICH ROWS came +// back: metadata audit history returned the oldest events (never an object's +// recent changes) and global search returned the stalest matches. `#4720` +// restored those two sites, `#4721` closed the external (REST/RPC) callers with +// a strict schema plus an ingress normalizer, and this rule is the third leg — +// it stops the erasure regrowing on the internal side. +const ENGINE_QUERY_READ_METHODS = ['find', 'findOne', 'count', 'aggregate']; + +// Exported so `scripts/check-query-options-erasure-ratchet.mjs` measures the +// SAME surface this rule blocks. The ratchet lifts these to count the test-side +// residual; the rule itself never runs on them. +// +// The first cut is deliberately non-test only (the 08-03 triage on #4918). Test +// code holds the large majority of the erasures, and an unknown share of those +// are legitimate: a test whose SUBJECT is off-contract engine input (see +// `engine-unknown-option.test.ts`, `engine-wire-alias-reject.test.ts`) has to +// erase the type to construct input `tsc` would otherwise refuse. A blocking +// rule there would fight the tests that prove the contract is enforced, so the +// test surface is held by a COUNT instead — see the ratchet. +export const QUERY_OPTIONS_TEST_GLOBS = [ + '**/*.test.{ts,tsx,mts,cts}', + '**/*.spec.{ts,tsx,mts,cts}', +]; + +// The rule's own id, exported so the ratchet identifies this rule's reports +// exactly rather than by message text. (The slot-lookup ratchet matches on +// message because that rule shares `no-restricted-syntax` with three others; +// this one is a dedicated rule, so the id is available and is stricter.) +export const QUERY_OPTIONS_RULE_ID = 'query-options/no-any-erasure'; + +export const QUERY_OPTIONS_ANY_MESSAGE = + 'Do not erase an engine query-options value to `any` — not as `find(obj, { … } ' + + 'as any)`, not as an `orderBy: … as any`, and not as a `const opts: any` that is ' + + 'then passed as the options argument. `EngineQueryOptions` (and `QueryAST` on the ' + + 'driver side) already declare every key these methods read, and for an internal ' + + 'caller `tsc` is the ONLY channel that enforces them: the protocol\'s ingress ' + + 'normalizer does not run on a direct engine call, and the options schemas are not ' + + '`.strict()`, so an unknown key is silently DROPPED, never rejected. That is #4674 ' + + '— two queries sorted by `direction` (IReportService\'s vocabulary) instead of ' + + '`order` (SortNodeSchema\'s), both with a `limit`, so both quietly returned the ' + + 'OLDEST rows: audit history that never showed an object\'s recent changes, and a ' + + 'global search that truncated away the freshly-edited records. The declared type ' + + 'would have rejected `direction` at the call site; the erasure is the only reason ' + + 'it compiled. Type the value instead (`const opts: EngineQueryOptions = { … }`, or ' + + 'just drop the assertion — these signatures already infer). If the value is ' + + 'DELIBERATELY off-contract — a test asserting the engine REJECTS an unknown option ' + + '— write `as unknown as EngineQueryOptions`: that names the contract being ' + + 'bypassed, keeps the rest of the call type-checked, and greps as an intentional ' + + 'act, none of which a bare `as any` does. See issues #4674, #4720, #4721, #4918.'; + +// [#4918] The unswept residual, grandfathered BY FILE from +// `scripts/query-options-erasure-baseline.json` — same mechanism, and same +// reasoning, as SLOT_LOOKUP_UNSWEPT above: `pnpm lint` runs with +// `--no-inline-config`, so the escape has to live in config, and one shrinking +// counted list is the ratchet made visible. An `ignores` entry silences the +// WHOLE file, which is exactly why the baseline carries per-file COUNTS and +// `pnpm check:query-options-erasure` enforces them. +// +// ⛔ Do NOT sweep these sites in the same PR that touches this rule. Part of the +// residual is a real type boundary (`hookContext.input.options`, the metadata +// loader's `Record` query bag) and needs the boundary type +// written, not the assertion deleted — a separate batch. +const QUERY_OPTIONS_UNSWEPT = Object.keys(JSON.parse( + readFileSync(new URL('./scripts/query-options-erasure-baseline.json', import.meta.url), 'utf8'), +).nonTest); + +const queryOptionsPlugin = { + rules: { + 'no-any-erasure': { + meta: { + type: 'problem', + docs: { description: 'Ban erasing an engine query-options value to `any`.' }, + schema: [], + messages: { erased: QUERY_OPTIONS_ANY_MESSAGE }, + }, + create(context) { + const methods = new Set(ENGINE_QUERY_READ_METHODS); + + /** + * True when `node` is, or wraps, an `any` assertion. + * + * Walks the whole assertion chain rather than testing the outermost + * node, so `{ … } as any as EngineQueryOptions` is caught too: that + * spelling checks the literal against nothing and then re-labels the + * result with the contract, which erases the keys exactly as `as any` + * does while reading as if it were typed. `as unknown as X` is NOT + * matched, on purpose — see the message. + */ + const erasesToAny = (node) => { + for (let cur = node; cur; cur = cur.expression) { + if (cur.type === 'TSAsExpression' || cur.type === 'TSTypeAssertion') { + if (cur.typeAnnotation?.type === 'TSAnyKeyword') return true; + continue; + } + if (cur.type === 'TSNonNullExpression') continue; + return false; + } + return false; + }; + + /** + * True when `name` resolves, in scope, to a local VARIABLE declared + * `: any` — the split form (`const opts: any = { … }` … `find(o, opts)`) + * that #4674's global-search site actually used. + * + * Scope analysis, not a name heuristic: a rule keyed on the identifier's + * spelling would flag every `const options: any` in the repo whether or + * not it ever reaches a query, and miss the ones spelled anything else. + * Deliberately restricted to variable declarations — an `: any` + * PARAMETER forwarded into a query is a different (and much larger, + * mostly test-double) population, out of this cut's scope. + */ + const declaredAnyVariable = (name, node) => { + for (let scope = context.sourceCode.getScope(node); scope; scope = scope.upper) { + const variable = scope.variables.find((v) => v.name === name); + if (!variable) continue; + return variable.defs.some( + (d) => + d.node?.type === 'VariableDeclarator' && + d.node.id?.typeAnnotation?.typeAnnotation?.type === 'TSAnyKeyword', + ); + } + return false; + }; + + return { + CallExpression(node) { + if (node.callee?.type !== 'MemberExpression') return; + const property = node.callee.property; + if (property?.type !== 'Identifier' || !methods.has(property.name)) return; + + node.arguments.forEach((argument, index) => { + // Argument 0 is the object/table NAME on every one of these + // signatures; the options bags are 1 (the query) and 2 + // (`BaseEngineOptions` / `DriverOptions`). Starting at 1 is also + // what keeps `Array.prototype.find(cb)` — same method name, + // callback at index 0 — out of the rule entirely. + if (index < 1 || !argument) return; + if (erasesToAny(argument)) { + context.report({ node: argument, messageId: 'erased' }); + return; + } + if (argument.type === 'Identifier' && declaredAnyVariable(argument.name, node)) { + context.report({ node: argument, messageId: 'erased' }); + } + }); + }, + + // `orderBy` is scoped in by name because it is the key #4674 was + // actually wrong about, and it is erased one level below the argument + // — `...(ast.orderBy ? { orderBy: ast.orderBy as any } : {})` sits + // inside an otherwise-typed options literal, so the argument-position + // check above cannot see it. `SortNodeSchema` is the shape everywhere + // this key appears. + Property(node) { + if (node.computed) return; + const key = node.key; + const isOrderBy = + (key?.type === 'Identifier' && key.name === 'orderBy') || + (key?.type === 'Literal' && key.value === 'orderBy'); + if (!isOrderBy) return; + if (erasesToAny(node.value)) { + context.report({ node: node.value, messageId: 'erased' }); + } + }, + }; + }, + }, + }, +}; + export default [ { files: ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}'], @@ -393,4 +585,48 @@ export default [ ], }, }, + // issue #4918 — engine query-options `any`-erasure guard. Rationale and the + // #4674 cost are on `QUERY_OPTIONS_ANY_MESSAGE` above. + // + // This is a dedicated PLUGIN rule and not three more `no-restricted-syntax` + // selectors, for two reasons that both matter: + // + // 1. Flat config does not MERGE rule options. A second block setting + // `no-restricted-syntax` over `packages/**` would REPLACE the + // slot-lookup block's selector list for every file both blocks match — + // silently deleting that rule. The two guards also need independent + // `ignores` (their unswept sets are different files), which one shared + // block cannot give them. + // 2. The split form needs SCOPE analysis to resolve an identifier to its + // declaration, which esquery cannot express — the same reason + // `slot-lookup/no-any-assignment` exists. Scope analysis needs no type + // information, so this still runs in the plain (untyped) lint pass. + // + // KNOWN RESIDUAL, stated rather than implied: an erasure that happens through + // a typed indirection — a helper declared `(o, q?: any) => engine.find(o, q)`, + // or a wrapper whose own return type is `Promise` — erases the contract + // just as effectively and this rule cannot see it (an `: any` PARAMETER + // forwarded into a query is a real shape, ~50 sites, almost all of them test + // doubles; judging it needs the call graph, not one file's scopes). Same + // boundary as the slot-lookup rule's own KNOWN RESIDUAL, and the same answer: + // it belongs to a typed-lint pass, not here. + { + files: ['packages/**/*.{ts,tsx,mts,cts}'], + ignores: [ + '**/node_modules/**', + '**/dist/**', + // First cut is non-test code (08-03 triage). The ratchet lifts this and + // holds the test residual to a count instead. + ...QUERY_OPTIONS_TEST_GLOBS, + // Pre-existing sites, grandfathered by file and counted — see + // QUERY_OPTIONS_UNSWEPT and `pnpm check:query-options-erasure`. + ...QUERY_OPTIONS_UNSWEPT, + ], + languageOptions: { + parser: tsParser, + parserOptions: { ecmaVersion: 'latest', sourceType: 'module' }, + }, + plugins: { 'query-options': queryOptionsPlugin }, + rules: { 'query-options/no-any-erasure': 'error' }, + }, ]; diff --git a/package.json b/package.json index bc49a819f4..f7e8e003b0 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "check:org-identifier": "node scripts/check-org-identifier.mjs", "check:authz-resolver": "node scripts/check-single-authz-resolver.mjs --self-test && node scripts/check-single-authz-resolver.mjs", "check:slot-lookup": "node scripts/check-slot-lookup-ratchet.mjs", + "check:query-options-erasure": "node scripts/check-query-options-erasure-ratchet.mjs --self-test && node scripts/check-query-options-erasure-ratchet.mjs", "check:service-providers": "node scripts/check-service-providers.mjs", "check:route-envelope": "node scripts/check-route-envelope.mjs --self-test && node scripts/check-route-envelope.mjs", "check:error-code-casing": "node scripts/check-error-code-casing.mjs --self-test && node scripts/check-error-code-casing.mjs", diff --git a/scripts/check-query-options-erasure-ratchet.mjs b/scripts/check-query-options-erasure-ratchet.mjs new file mode 100644 index 0000000000..e0289c7074 --- /dev/null +++ b/scripts/check-query-options-erasure-ratchet.mjs @@ -0,0 +1,425 @@ +#!/usr/bin/env node +// check-query-options-erasure-ratchet — the #4918 guard, enforced. +// +// `eslint.config.mjs` bans erasing an engine query-options value to `any` +// (`query-options/no-any-erasure`, rationale on QUERY_OPTIONS_ANY_MESSAGE +// there). Two populations sit outside what `pnpm lint` can block on its own, +// and this script is what stops each of them from going dark: +// +// • NON-TEST residual. The files that still hold pre-existing sites are +// grandfathered by path in `scripts/query-options-erasure-baseline.json`. +// An ESLint `ignores` entry silences the WHOLE file, so a new erasure added +// to a listed file would ride the old entry in total silence — the exact +// move the slot-lookup ratchet was written to stop (#4251). So the baseline +// carries per-file COUNTS, measured here with the grandfathering lifted. +// +// • TEST residual. The 08-03 triage scoped the blocking rule to non-test code: +// an unknown share of the test sites are LEGITIMATE, because a test whose +// subject is off-contract engine input has to erase the type to construct +// input `tsc` would refuse. Those are held by one aggregate decrease-only +// number instead, so the surface is measured and cannot grow unnoticed +// without a per-file ratchet going red on a legitimate rejection test. +// +// It fails when: +// • a non-test file NOT in the baseline reports a site (that already fails +// `pnpm lint` — reported here too so one command explains the picture), or +// • a baselined file's count INCREASES (new erasure hiding behind an old +// entry — the invisible move), or +// • a baselined file's count DECREASED, or the file is clean/gone (progress!) +// — run with --update to ratchet the baseline down and commit it, or +// • a file was ADDED to the baseline relative to the merge base with main +// (the grandfather list is not a mute button), or +// • the aggregate TEST count moved in either direction: up is a new erasure, +// down is un-ratcheted progress. A ceiling left above reality silently +// licenses that many new erasures, which is how a ratchet stops meaning +// anything. +// +// node scripts/check-query-options-erasure-ratchet.mjs [--update] [--self-test] +// +// The counts are produced by running ESLint itself over the real config with +// the relevant `ignores` lifted, and reports are matched by the rule's exact +// id. The counter therefore cannot drift from the rule: change the rule and +// this re-measures against it. +import { execFileSync } from 'node:child_process'; +import { readFileSync, writeFileSync } from 'node:fs'; +import { dirname, resolve, relative } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { ESLint } from 'eslint'; + +import eslintConfig, { + QUERY_OPTIONS_RULE_ID, + QUERY_OPTIONS_TEST_GLOBS, + QUERY_OPTIONS_ANY_MESSAGE, +} from '../eslint.config.mjs'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const repoRoot = resolve(__dirname, '..'); +const BASELINE_PATH = 'scripts/query-options-erasure-baseline.json'; +const LINT_TARGET = 'packages/**/*.{ts,tsx,mts,cts}'; + +const carriesRule = (entry) => entry?.rules?.[QUERY_OPTIONS_RULE_ID] !== undefined; + +/** + * The config with a chosen set of `ignores` entries dropped from the block that + * carries the rule. Every other config entry passes through untouched, so the + * run stays byte-identical to `pnpm lint` in all other respects. + */ +function measuringConfig(drop) { + return eslintConfig.map((entry) => + carriesRule(entry) + ? { ...entry, ignores: (entry.ignores ?? []).filter((p) => !drop.has(p)) } + : entry, + ); +} + +async function measure(drop, targets = [LINT_TARGET]) { + const eslint = new ESLint({ + cwd: repoRoot, + overrideConfigFile: true, + baseConfig: measuringConfig(drop), + // Match the root `lint` script: this repo lints with --no-inline-config on + // purpose, so an eslint-disable comment must not shrink a count here either. + allowInlineConfig: false, + }); + const results = await eslint.lintFiles(targets); + const counts = {}; + for (const result of results) { + const hits = result.messages.filter((m) => m.ruleId === QUERY_OPTIONS_RULE_ID).length; + if (hits > 0) counts[relative(repoRoot, result.filePath).replace(/\\/g, '/')] = hits; + } + return counts; +} + +const sum = (counts) => Object.values(counts).reduce((a, b) => a + b, 0); +const sortKeys = (counts) => + Object.fromEntries(Object.entries(counts).sort(([a], [b]) => a.localeCompare(b))); + +/** + * Every failure this gate can report, as a pure function of the four measured + * facts. Kept pure so --self-test can drive it in BOTH directions without + * needing a repo in a particular state — the comparison logic is the half of + * this script that a green run over a clean tree cannot exercise at all. + */ +export function diffRatchet({ baseline, current, testCeiling, testSites, addedBaselineKeys }) { + const errors = []; + + for (const [file, count] of Object.entries(current)) { + const allowed = baseline[file]; + if (allowed === undefined) { + errors.push( + `${file}: NEW engine query-options erasure (${count} site(s)). Type the ` + + `options (\`EngineQueryOptions\` / \`QueryAST\`) instead of erasing them to ` + + `\`any\` — see eslint.config.mjs and issue #4918. This file is not ` + + `grandfathered, and the baseline never grows.`, + ); + } else if (count > allowed) { + errors.push( + `${file}: erasure count grew ${allowed} → ${count}. The file is grandfathered ` + + `for its EXISTING sites only; new ones must carry the declared options type.`, + ); + } + } + + for (const [file, allowed] of Object.entries(baseline)) { + const now = current[file]; + if (now === undefined) { + errors.push( + `${file}: baselined file is clean/gone (was ${allowed}) — ratchet DOWN: run ` + + `\`pnpm check:query-options-erasure --update\` and commit the baseline.`, + ); + } else if (now < allowed) { + errors.push( + `${file}: erasure count fell ${allowed} → ${now} — ratchet DOWN: run ` + + `\`pnpm check:query-options-erasure --update\` and commit the baseline.`, + ); + } + } + + // The key set must only ever SHRINK. Counts alone cannot see the last move: a + // genuinely-erasing NEW file added to the baseline matches its own count and + // sails through, which would turn the grandfather list into a general-purpose + // mute button. + for (const file of addedBaselineKeys ?? []) { + errors.push( + `${file}: ADDED to the baseline. The grandfather list is not a mute button — it ` + + `only ever shrinks. Type this file's query options instead; see issue #4918.`, + ); + } + + if (testSites > testCeiling) { + errors.push( + `test surface grew ${testCeiling} → ${testSites} site(s). Tests are outside the ` + + `blocking rule, not outside the count. Either type the options, or — if the ` + + `input is DELIBERATELY off-contract (a test asserting the engine rejects an ` + + `unknown option) — write \`as unknown as EngineQueryOptions\`, which names the ` + + `contract being bypassed, keeps the rest of the call checked, and is not counted ` + + `here. Raising this number is a reviewed edit, not a remedy.`, + ); + } else if (testSites < testCeiling) { + errors.push( + `test surface fell ${testCeiling} → ${testSites} site(s) — ratchet DOWN: run ` + + `\`pnpm check:query-options-erasure --update\` and commit the baseline. A ceiling ` + + `left above reality silently licenses that many new erasures.`, + ); + } + + return errors; +} + +/** Baseline keys that are not present at the merge base with `main`. */ +function baselineKeysAddedSinceMergeBase(baselineKeys) { + try { + const git = (...args) => + execFileSync('git', args, { + cwd: repoRoot, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + }).trim(); + let base; + for (const ref of ['origin/main', 'main']) { + try { base = git('merge-base', 'HEAD', ref); break; } catch { /* try the next ref */ } + } + if (!base) return null; + const previous = JSON.parse(git('show', `${base}:${BASELINE_PATH}`)); + const known = new Set(Object.keys(previous.nonTest ?? {})); + return { base: base.slice(0, 7), added: baselineKeys.filter((f) => !known.has(f)) }; + } catch { + // No git, a shallow clone without the base, or the baseline is new on this + // branch (`git show` fails). Reported by the caller rather than passed over + // — a check that cannot run must not read as a check that passed. + return null; + } +} + +// --------------------------------------------------------------------------- +// --self-test + +async function selfTest() { + const failures = []; + const assert = (cond, msg) => { if (!cond) failures.push(msg); }; + + // ── 1. The rule, in both directions, over synthetic sources. ────────────── + // + // A gate that has only ever been green cannot be told apart from a gate that + // matches nothing (#4690), so every shape is proved to REPORT and its + // canonical counterpart proved to stay SILENT. + const drop = new Set([...QUERY_OPTIONS_TEST_GLOBS]); + const eslint = new ESLint({ + cwd: repoRoot, + overrideConfigFile: true, + baseConfig: measuringConfig(drop), + allowInlineConfig: false, + }); + const hits = async (code, filePath = 'packages/objectql/src/__selftest__.ts') => { + const [result] = await eslint.lintText(code, { filePath, warnIgnored: false }); + return (result?.messages ?? []).filter((m) => m.ruleId === QUERY_OPTIONS_RULE_ID).length; + }; + + const reports = [ + ['argument 1, object literal', "await e.find('o', { where: {}, orderBy: [] } as any);"], + ['argument 1, identifier', 'await e.findOne(t, query as any);'], + ['argument 2, member expression', 'await d.find(o, ast, ctx.input.options as any);'], + ['count()', "await e.count('o', { where: {} } as any);"], + ['aggregate()', "await e.aggregate('o', { aggregations: [] } as any);"], + ['angle-bracket assertion', "await e.find('o', opts);"], + [ + 'laundered chain — `as any as Contract` erases exactly as `as any` does', + "await e.find('o', { direction: 'desc' } as any as EngineQueryOptions);", + ], + [ + 'split declaration — the shape #4674 global search actually used', + "const opts: any = { where: w }; await e.find('o', opts);", + ], + ['orderBy one level down', "await e.find('o', { ...(a.orderBy ? { orderBy: a.orderBy as any } : {}) });"], + ['orderBy as a string key', "const q = { 'orderBy': sort as any };"], + ]; + for (const [name, code] of reports) { + assert((await hits(code)) >= 1, `expected a report for ${name}: ${code}`); + } + + const silent = [ + ['typed options need no assertion', "await e.find('o', { where: {}, orderBy: [{ field: 'a', order: 'desc' }] });"], + ['a typed local', "const opts: EngineQueryOptions = { where: w }; await e.find('o', opts);"], + [ + 'the deliberate spelling is the sanctioned escape', + "await e.find('o', { nope: 1 } as unknown as EngineQueryOptions);", + ], + ['Array.prototype.find — callback at index 0', 'rows.find((r) => r.id === id as any);'], + ['a cast RESULT is not an options erasure', "const rows = (await e.find('o', { where: w })) as any;"], + ['argument 0 is the object name, never options', 'await e.find(name as any, { where: w });'], + ['an unrelated `orderBy` on a non-any assertion', "const q = { orderBy: sort as SortNode[] };"], + ['a computed key that merely spells orderBy', 'const q = { [orderBy]: sort as any };'], + ['an `: any` local that never reaches a query', 'const opts: any = { where: w }; use(opts);'], + ['a shadowed inner binding that is typed', 'const opts: any = 1; { const opts: EngineQueryOptions = q; e.find(o, opts); }'], + ]; + for (const [name, code] of silent) { + assert((await hits(code)) === 0, `expected NO report for ${name}: ${code}`); + } + + // The grandfathering channel is real: the same source is silent on a + // baselined path and loud on a path that is not baselined. + { + const baselined = Object.keys( + JSON.parse(readFileSync(resolve(repoRoot, BASELINE_PATH), 'utf8')).nonTest, + )[0]; + const code = "await e.find('o', { where: {} } as any);"; + assert( + baselined === undefined || (await hits(code, baselined)) === 0, + `a baselined path must be silent under the blocking config (${baselined})`, + ); + assert( + (await hits(code, 'packages/objectql/src/__selftest_not_baselined__.ts')) >= 1, + 'a non-baselined path must report', + ); + } + + // The test-glob exclusion is real, and it is what the ratchet lifts. + { + const code = "await e.find('o', { where: {} } as any);"; + const blocking = new ESLint({ + cwd: repoRoot, + overrideConfigFile: true, + baseConfig: eslintConfig, + allowInlineConfig: false, + }); + const [result] = await blocking.lintText(code, { + filePath: 'packages/objectql/src/__selftest__.test.ts', + warnIgnored: false, + }); + const blocked = (result?.messages ?? []).filter((m) => m.ruleId === QUERY_OPTIONS_RULE_ID).length; + assert(blocked === 0, 'a *.test.ts path must NOT be blocked by the rule (first cut is non-test)'); + assert( + (await hits(code, 'packages/objectql/src/__selftest__.test.ts')) >= 1, + 'the same *.test.ts path MUST be counted once the ratchet lifts the test globs', + ); + } + + assert( + /4674/.test(QUERY_OPTIONS_ANY_MESSAGE) && /as unknown as/.test(QUERY_OPTIONS_ANY_MESSAGE), + 'the rule message must carry the #4674 cost and the sanctioned deliberate spelling', + ); + + // ── 2. The ratchet comparison, in both directions. ──────────────────────── + const base = { 'a.ts': 2, 'b.ts': 1 }; + const cases = [ + ['identical + ceiling met is clean', { baseline: base, current: { ...base }, testCeiling: 10, testSites: 10, addedBaselineKeys: [] }, 0], + ['a new file fails', { baseline: base, current: { ...base, 'c.ts': 1 }, testCeiling: 10, testSites: 10, addedBaselineKeys: [] }, 1], + ['growth in a baselined file fails', { baseline: base, current: { 'a.ts': 3, 'b.ts': 1 }, testCeiling: 10, testSites: 10, addedBaselineKeys: [] }, 1], + ['a fall must be ratcheted down', { baseline: base, current: { 'a.ts': 1, 'b.ts': 1 }, testCeiling: 10, testSites: 10, addedBaselineKeys: [] }, 1], + ['a cleaned file must be dropped', { baseline: base, current: { 'a.ts': 2 }, testCeiling: 10, testSites: 10, addedBaselineKeys: [] }, 1], + ['a key added to the baseline fails', { baseline: base, current: { ...base }, testCeiling: 10, testSites: 10, addedBaselineKeys: ['b.ts'] }, 1], + ['test-surface growth fails', { baseline: base, current: { ...base }, testCeiling: 10, testSites: 11, addedBaselineKeys: [] }, 1], + ['test-surface shrink must be ratcheted down', { baseline: base, current: { ...base }, testCeiling: 10, testSites: 9, addedBaselineKeys: [] }, 1], + ]; + for (const [name, input, expected] of cases) { + const got = diffRatchet(input).length; + assert(got === expected, `diffRatchet: ${name} — expected ${expected} error(s), got ${got}`); + } + + // A missing config block must ABORT, never report clean. + assert(eslintConfig.some(carriesRule), 'the config must carry the query-options rule'); + + if (failures.length > 0) { + console.error(`✗ self-test (${failures.length} failure(s)):\n`); + for (const f of failures) console.error(` • ${f}`); + process.exit(1); + } + console.log( + `✓ self-test: ${reports.length} reporting shape(s), ${silent.length} silent counterpart(s), ` + + `grandfathering + test-glob channels proved in both directions, ${cases.length} ratchet case(s).`, + ); +} + +// --------------------------------------------------------------------------- +// main + +if (process.argv.includes('--self-test')) { + await selfTest(); + process.exit(0); +} + +if (!eslintConfig.some(carriesRule)) { + console.error( + `check-query-options-erasure-ratchet: no config block carries \`${QUERY_OPTIONS_RULE_ID}\`.\n` + + 'The rule was renamed or removed without updating QUERY_OPTIONS_RULE_ID — refusing\n' + + 'to report "clean" for a rule that is no longer being measured.', + ); + process.exit(2); +} + +const update = process.argv.includes('--update'); +const baselineFile = JSON.parse(readFileSync(resolve(repoRoot, BASELINE_PATH), 'utf8')); +const baseline = baselineFile.nonTest ?? {}; +const testCeiling = baselineFile.testSurface?.sites; + +if (typeof testCeiling !== 'number' && !update) { + console.error( + `check-query-options-erasure-ratchet: ${BASELINE_PATH} has no numeric ` + + '`testSurface.sites`. Refusing to report clean with half the surface unmeasured.', + ); + process.exit(2); +} + +// Two runs, one per population. The split is done by ESLint against the very +// globs the rule uses, so there is no second definition of "is this a test +// file" for the two halves to drift apart on. +const nonTest = sortKeys(await measure(new Set(Object.keys(baseline)))); +const everything = sortKeys(await measure(new Set([...Object.keys(baseline), ...QUERY_OPTIONS_TEST_GLOBS]))); +const testOnly = sortKeys( + Object.fromEntries(Object.entries(everything).filter(([file]) => !(file in nonTest))), +); +const testSites = sum(testOnly); + +if (update) { + const next = { + ...baselineFile, + nonTest, + testSurface: { ...(baselineFile.testSurface ?? {}), sites: testSites }, + }; + writeFileSync(resolve(repoRoot, BASELINE_PATH), JSON.stringify(next, null, 2) + '\n'); + console.log( + `query-options-erasure baseline updated: ${sum(nonTest)} non-test site(s) in ` + + `${Object.keys(nonTest).length} file(s); test surface ${testSites} site(s) in ` + + `${Object.keys(testOnly).length} file(s).`, + ); + process.exit(0); +} + +const monotonicity = baselineKeysAddedSinceMergeBase(Object.keys(baseline)); +const errors = diffRatchet({ + baseline, + current: nonTest, + testCeiling, + testSites, + addedBaselineKeys: monotonicity?.added ?? [], +}); + +if (errors.length > 0) { + console.error(`✗ query-options-erasure ratchet (${errors.length} problem(s)):\n`); + for (const e of errors) console.error(` • ${e}`); + console.error( + `\nUnswept: ${sum(nonTest)} non-test site(s) in ${Object.keys(nonTest).length} file(s), ` + + `plus ${testSites} in test code. Sweeping is a separate batch — part of the residual ` + + `needs a boundary type WRITTEN (objectql's \`hookContext.input.options\`, the metadata ` + + `loader's query bag), not the assertion deleted. See issue #4918.`, + ); + process.exit(1); +} + +console.log( + `✓ query-options-erasure ratchet holds: ${sum(nonTest)} unswept non-test site(s) in ` + + `${Object.keys(nonTest).length} file(s), none new. Every other non-test file under ` + + `packages/ is covered by \`pnpm lint\`.`, +); +console.log( + ` test surface: ${testSites} site(s) in ${Object.keys(testOnly).length} file(s) — at the ` + + `ceiling, outside the blocking rule by the #4918 triage (a rejection test must be able ` + + `to build off-contract input).`, +); +console.log( + monotonicity + ? ` baseline key set verified against ${monotonicity.base}: no files added.` + : ` NOT verified: could not read the baseline at the merge base with main (no git, ` + + `shallow clone, or the baseline is new here), so "no files added" is unchecked this run.`, +); diff --git a/scripts/query-options-erasure-baseline.json b/scripts/query-options-erasure-baseline.json new file mode 100644 index 0000000000..1b195743a3 --- /dev/null +++ b/scripts/query-options-erasure-baseline.json @@ -0,0 +1,57 @@ +{ + "$comment": [ + "Unswept engine query-options `any`-erasure sites (#4918), enforced by", + "scripts/check-query-options-erasure-ratchet.mjs.", + "", + "`nonTest` keys ARE the `ignores` of the `query-options/no-any-erasure` block in", + "eslint.config.mjs, and its values are the per-file counts this checker enforces.", + "That coupling is the point: an `ignores` entry silences the WHOLE file, so a bare", + "path list would let NEW erasures ride an existing entry in total silence (the", + "#4251 lesson, paid for by the slot-lookup ratchet). Counts fail that move.", + "", + "SHRINK-ONLY, in both dimensions. The key set only ever loses files (checked", + "against the merge base with main — adding a file here is not a sanctioned move,", + "it is a mute button), and a count that FALLS must be ratcheted down in the same", + "PR: run `pnpm check:query-options-erasure --update` and commit. A ceiling left", + "above reality silently licenses that many new erasures.", + "", + "`testSurface.sites` is the aggregate test-code residual, held as one", + "decrease-only number rather than per-file. The 08-03 triage scoped the first cut", + "to non-test code because an unknown share of the test sites are LEGITIMATE: a", + "test whose subject is off-contract engine input (engine-unknown-option.test.ts,", + "engine-wire-alias-reject.test.ts, sqlite-wasm-out-of-contract-filter-input.test.ts)", + "must erase the type to build input `tsc` would refuse. A per-file ratchet there", + "would go red on a legitimate new rejection test with no honest remedy; the", + "aggregate gives one — spell the deliberate case `as unknown as EngineQueryOptions`,", + "which names the contract being bypassed and leaves this count alone.", + "", + "⛔ Sweeping the residual is NOT this gate's PR. Part of it is a real type boundary", + "(objectql's `hookContext.input.options`, the metadata loader's", + "`Record` query bag) that needs the boundary type WRITTEN, not the", + "assertion deleted. Measured on the branch point e900015cd (2026-08-05)." + ], + "nonTest": { + "packages/cloud-connection/src/marketplace-install-local-plugin.ts": 2, + "packages/core/src/security/resolve-authz-context.ts": 1, + "packages/metadata-protocol/src/protocol.ts": 6, + "packages/metadata-protocol/src/seed-loader.ts": 3, + "packages/metadata/src/loaders/database-loader.ts": 6, + "packages/objectql/src/engine.ts": 13, + "packages/plugins/plugin-approvals/src/approval-service.ts": 10, + "packages/plugins/plugin-approvals/src/approver-org-scope.ts": 2, + "packages/plugins/plugin-approvals/src/lifecycle-hooks.ts": 4, + "packages/plugins/plugin-auth/src/admin-import-users.ts": 1, + "packages/plugins/plugin-auth/src/auth-manager.ts": 14, + "packages/plugins/plugin-hono-server/src/current-user-endpoints.ts": 6, + "packages/plugins/plugin-sharing/src/share-link-routes.ts": 3, + "packages/plugins/plugin-sharing/src/share-link-service.ts": 5, + "packages/plugins/plugin-webhooks/src/bootstrap-declared-webhooks.ts": 1, + "packages/runtime/src/action-execution.ts": 1, + "packages/runtime/src/domains/share-links.ts": 3, + "packages/runtime/src/http-dispatcher.ts": 1, + "packages/services/service-settings/src/settings-service.ts": 2 + }, + "testSurface": { + "sites": 267 + } +}