Skip to content

Commit f06d8ed

Browse files
fix(tooling): teach check-error-code-casing the || / ?? fallback slot, and qualify its verdict (#10760)
The four CODE_POSITION_PATTERNS all anchor the string literal immediately after the position token, so an intervening expression makes the literal invisible to the whole set. Two live wire-visible codes shipped through that gap while the gate printed an unqualified 'no lowercase error codes'. - fifth pattern reaching our default through an ||/?? chain, bounded so a match cannot leap into a neighbouring property's fallback; - KNOWN_LOWERCASE_CODES, shrink-only, carrying the two codes whose rename is owned by #10716 (services lane) — a stale entry fails; - the verdict now states what the run could not read (#10501 precedent). Part of #10658 Co-authored-by: Claude <jack@objectstack.ai>
1 parent 01c5032 commit f06d8ed

1 file changed

Lines changed: 178 additions & 8 deletions

File tree

scripts/check-error-code-casing.mjs

Lines changed: 178 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,79 @@ const CODE_POSITION_PATTERNS = [
113113
{ name: 'comparison', re: /\bcode\s*(?:===|!==)\s*'([a-z][a-z0-9_]*)'/g },
114114
// literal-union type: code?: 'x' | 'y' (the one that breaks a consumer's dts)
115115
{ name: 'union-type', re: /\bcode\??\s*:\s*'([a-z][a-z0-9_]*)'\s*\|/g },
116+
// [#10658] the OUR-DEFAULT slot of a fallback chain:
117+
// code: parsed?.code || 'verify_domain_failed'
118+
// err.code = e?.code ?? 'lookup_failed'
119+
//
120+
// The four patterns above all anchor the quote DIRECTLY after the position
121+
// token, so any intervening expression makes the literal invisible to the
122+
// whole set — silently, and then the run prints a total that reads as
123+
// complete. Two live wire codes shipped through exactly that gap.
124+
//
125+
// Where the line is drawn, and why it cannot start flagging a pass-through:
126+
// this pattern still only ever captures a STRING LITERAL, and a literal in
127+
// our source is by construction ours. A vendor code "passing through" is a
128+
// RUNTIME value (`parsed?.code`, `err.code`, a variable) — it has no literal
129+
// for any pattern here to capture, before or after this widening. So the
130+
// operand this reaches is only ever the default WE author. (A vendor's
131+
// spelling hard-coded as our default — `parsed?.code || 'invalid_grant'` — is
132+
// a real D1 violation: it is the code OUR failing request answers with.)
133+
//
134+
// The gap class is what keeps the match inside one property's value. It
135+
// admits an operand chain (identifiers, member/optional access, calls,
136+
// indexes, further `||`/`??`) and refuses `, ; : { } =` and every quote, so a
137+
// match cannot leap out of `code:` into a NEIGHBOUR's fallback — the real
138+
// false positive here is `{ code: a.code, message: m || 'lower' }`, and that
139+
// comma is what stops it. The length bound is a runaway guard; both are
140+
// pinned in --self-test.
141+
{
142+
name: 'fallback',
143+
re: /(?:\bcode\s*\??\s*:|\.code\s*=(?!=))\s*(?![`'"])[\w$.?!()[\]|&\s]{0,80}?(?:\|\||\?\?)\s*'([a-z][a-z0-9_]*)'/g,
144+
},
116145
];
117146

147+
/**
148+
* ⛔ SHRINK-ONLY. Lowercase codes that are ALREADY ON THE WIRE and whose rename
149+
* belongs to another lane's card, keyed `<file>::<literal>` so a line shift
150+
* cannot invalidate an entry.
151+
*
152+
* This list exists for one reason and admits nothing else: widening the
153+
* recognizer above made this gate able to see codes that shipped while it was
154+
* blind. Renaming them is a WIRE change — a client matching on the old spelling
155+
* breaks, and one is pinned by name in
156+
* `packages/qa/dogfood/test/admin-route-nonadmin-refusal.dogfood.test.ts` — so
157+
* it is owned by the service that emits them, not by the tooling change that
158+
* revealed them.
159+
*
160+
* The list only ever shrinks. An entry that stops matching is a FAILURE (the
161+
* rename landed — delete the line), which is what keeps this from drifting into
162+
* an allowlist nobody re-reads. A NEW lowercase code never joins it: the gate
163+
* refuses that, and the remedy for a fresh finding is a registered SCREAMING
164+
* code, never a line here.
165+
*/
166+
const KNOWN_LOWERCASE_CODES = new Map([
167+
[
168+
'packages/plugins/plugin-auth/src/register-sso-provider.ts::request_domain_verification_failed',
169+
'wire-visible; rename owned by #10716 (services lane)',
170+
],
171+
[
172+
'packages/plugins/plugin-auth/src/register-sso-provider.ts::verify_domain_failed',
173+
'wire-visible; rename owned by #10716 (services lane); pinned by name in the dogfood suite',
174+
],
175+
]);
176+
177+
/** Split findings into the two the wire already carries and everything else. */
178+
export function partitionKnown(violations) {
179+
const known = [];
180+
const fresh = [];
181+
for (const v of violations) {
182+
(KNOWN_LOWERCASE_CODES.has(`${v.file}::${v.literal}`) ? known : fresh).push(v);
183+
}
184+
const reached = new Set(known.map((v) => `${v.file}::${v.literal}`));
185+
const stale = [...KNOWN_LOWERCASE_CODES.keys()].filter((k) => !reached.has(k)).sort();
186+
return { known, fresh, stale };
187+
}
188+
118189
function walk(dir, out = []) {
119190
for (const entry of readdirSync(dir)) {
120191
if (entry === 'node_modules' || entry === 'dist' || entry === '.turbo' || entry === 'coverage') continue;
@@ -128,7 +199,7 @@ function walk(dir, out = []) {
128199

129200

130201

131-
export function findViolations(src, file) {
202+
export function findViolations(src, file, stats = null) {
132203
const text = maskComments(src);
133204
const hits = [];
134205
for (const { name, re } of CODE_POSITION_PATTERNS) {
@@ -163,7 +234,12 @@ export function findViolations(src, file) {
163234
const rawLines = src.split('\n');
164235
const own = rawLines[lineNo - 1] ?? '';
165236
const prev = rawLines[lineNo - 2] ?? '';
166-
if (/adr0112-ok:\s*\S/.test(own) || /adr0112-ok:\s*\S/.test(prev)) continue;
237+
if (/adr0112-ok:\s*\S/.test(own) || /adr0112-ok:\s*\S/.test(prev)) {
238+
// Counted, not just skipped: a suppression this run APPLIED is part of
239+
// what the verdict line has to own up to.
240+
if (stats) stats.optOuts++;
241+
continue;
242+
}
167243
hits.push({ file, line: lineNo, literal, form: name });
168244
}
169245
}
@@ -190,6 +266,30 @@ function selfTest() {
190266
[`const plan = PlanSchema.parse({ code: 'pro_v1', features: [] });`, 0, 'license plan code is domain data'],
191267
[`records: [{ code: 'tech', name: 'Technology' }],`, 0, 'seed industry code is domain data'],
192268
[`{ code: 'required', message: 'x', target: 'email' }`, 0, "D6 via OData's target"],
269+
270+
// [#10658] The `||` / `??` fallback slot. Pinned as a PAIR with the direct
271+
// spelling of the same code: a recognizer that reached the new shape by
272+
// breaking the old one would pass a self-test that only pinned the new one.
273+
[`error: { code: 'verify_domain_failed', message }`, 1, 'direct spelling (the pair half that must not regress)'],
274+
[`error: { code: parsed?.code || 'verify_domain_failed', message }`, 1, 'our default behind an || fallback'],
275+
[`error: { code: parsed?.code ?? 'lookup_failed', message }`, 1, 'our default behind a ?? fallback'],
276+
[`const err = new Error(msg); (err as any).code = e?.code || 'lookup_failed';`, 1, 'fallback in the assignment position'],
277+
[`error: { code: a?.code || b?.code || 'chained_failed', message }`, 1, 'fallback at the end of a chain'],
278+
279+
// Reject side. A vendor code PASSING THROUGH is a runtime value, so it has
280+
// no literal to capture — that is the line, and it is why widening here
281+
// cannot start flagging one.
282+
[`return { status: resp.status, body: { error: { code: parsed?.code, message } } };`, 0, 'vendor code passing through has no literal'],
283+
[`error: { code: parsed?.code || 'VERIFY_DOMAIN_FAILED', message }`, 0, 'a SCREAMING default is compliant'],
284+
[`{ code: a.code, message: m || 'lower_thing' }`, 0, "a neighbour's fallback is not this code's value"],
285+
[`issues.push({ field: 'email', code: e?.code || 'invalid_email' });`, 0, 'D6 still wins through the fallback shape'],
286+
[`{ code: row.code || 'ok', message: 'x' }`, 0, 'NOT_CODES still applies through the fallback shape'],
287+
[`error: { code: e?.code || 'item_locked', message } // adr0112-ok: D6b persisted audit column`, 0, 'opt-out still applies through the fallback shape'],
288+
[
289+
`error: { code: a.b.c.${'d'.repeat(90)} || 'far_away_failed', message }`,
290+
0,
291+
'the gap is bounded: a runaway expression is a declared miss, not a leap',
292+
],
193293
];
194294
let failed = 0;
195295
for (const [src, want, label] of cases) {
@@ -199,31 +299,76 @@ function selfTest() {
199299
failed++;
200300
}
201301
}
302+
// [#10658] The shrink-only registry, in both directions. The second one is
303+
// the load-bearing half: when the owning card's rename lands, a stale line
304+
// must FAIL rather than sit there as a quiet allowlist entry.
305+
const SSO = 'packages/plugins/plugin-auth/src/register-sso-provider.ts';
306+
const row = (literal) => ({ file: SSO, line: 1, literal, form: 'fallback' });
307+
const partitionCases = [
308+
[[row('request_domain_verification_failed'), row('verify_domain_failed')], { known: 2, fresh: 0, stale: 0 }, 'both known rows still present'],
309+
[
310+
[row('request_domain_verification_failed'), row('verify_domain_failed'), row('brand_new_failure')],
311+
{ known: 2, fresh: 1, stale: 0 },
312+
'a NEW lowercase code is fresh, never absorbed by the list',
313+
],
314+
[[row('verify_domain_failed')], { known: 1, fresh: 0, stale: 1 }, 'a landed rename goes STALE and must fail'],
315+
[[], { known: 0, fresh: 0, stale: 2 }, 'an empty tree makes every entry stale'],
316+
];
317+
for (const [input, want, label] of partitionCases) {
318+
const got = partitionKnown(input);
319+
const shape = { known: got.known.length, fresh: got.fresh.length, stale: got.stale.length };
320+
if (shape.known !== want.known || shape.fresh !== want.fresh || shape.stale !== want.stale) {
321+
console.error(` ✗ self-test "${label}": expected ${JSON.stringify(want)}, got ${JSON.stringify(shape)}`);
322+
failed++;
323+
}
324+
}
325+
202326
if (failed) {
203327
console.error(`\n✗ check-error-code-casing self-test failed (${failed} case(s)).`);
204328
process.exit(1);
205329
}
206-
console.log(`✓ check-error-code-casing self-test: ${cases.length} cases pass.`);
330+
console.log(
331+
`✓ check-error-code-casing self-test: ${cases.length} recognizer case(s) + ${partitionCases.length} registry case(s) pass.`,
332+
);
207333
}
208334

209335
function main() {
210336
if (process.argv.includes('--self-test')) return selfTest();
211337

212338
const files = SCAN_ROOTS.flatMap((r) => walk(join(ROOT, r)));
339+
const stats = { optOuts: 0, exempt: 0 };
213340
const violations = [];
214341
for (const full of files) {
215342
const rel = relative(ROOT, full).split(sep).join('/');
216-
if (EXEMPT_FILES.has(rel)) continue;
217-
violations.push(...findViolations(readFileSync(full, 'utf8'), rel));
343+
if (EXEMPT_FILES.has(rel)) {
344+
stats.exempt++;
345+
continue;
346+
}
347+
violations.push(...findViolations(readFileSync(full, 'utf8'), rel, stats));
218348
}
219349

220-
if (violations.length === 0) {
221-
console.log(`✓ no lowercase error codes in ${files.length} scanned file(s) (ADR-0112).`);
350+
const { known, fresh, stale } = partitionKnown(violations);
351+
352+
if (fresh.length === 0 && stale.length === 0) {
353+
console.log(`✓ no unlisted lowercase error codes in ${files.length} scanned file(s) (ADR-0112).`);
354+
console.log(unreadable(stats, known));
222355
return;
223356
}
224357

358+
if (stale.length) {
359+
console.error(`\n✗ ${stale.length} stale KNOWN_LOWERCASE_CODES entry/entries:\n`);
360+
for (const key of stale) console.error(` ${key.replace('::', " '")}' — no longer present`);
361+
console.error(`
362+
Good news, and the list has to say so out loud: the rename landed, so DELETE
363+
each line above from KNOWN_LOWERCASE_CODES in this script. That list only ever
364+
shrinks, and a stale line is how it would have started drifting into an
365+
allowlist nobody re-reads.
366+
`);
367+
if (fresh.length === 0) process.exit(1);
368+
}
369+
225370
console.error(`\n✗ lowercase error-code literal(s) in a code position (ADR-0112 D1):\n`);
226-
for (const v of violations) {
371+
for (const v of fresh) {
227372
console.error(` ${v.file}:${v.line} '${v.literal}' (${v.form})`);
228373
}
229374
console.error(`
@@ -238,10 +383,35 @@ error.code is a closed set of SCREAMING_SNAKE values — StandardErrorCode
238383
If this literal is NOT an error.code — a field/param-addressed validator code
239384
(D6), a persisted column (D6b), or a diagnostics record shipped inside a 200
240385
(D6c) — add the file to EXEMPT_FILES in this script WITH its reason.
386+
387+
KNOWN_LOWERCASE_CODES is not a way out and this gate does not offer it: that
388+
registry only ever shrinks, it is closed to a code found here, and no new line
389+
is admitted to it. It holds codes that were already on the wire when this gate
390+
was still blind to their shape, each owned by the card that renames it.
241391
`);
392+
console.error(unreadable(stats, known));
242393
process.exit(1);
243394
}
244395

396+
/**
397+
* The other half of the fraction (#10501): a scan that reports only what it
398+
* FOUND renders a bounded read exactly like a complete one, and that is the
399+
* defect this gate shipped — an unqualified "no lowercase error codes" over a
400+
* tree carrying two. Every blindness below is deliberate and this run can count
401+
* it, so the verdict states it rather than implying none exists.
402+
*/
403+
function unreadable(stats, known) {
404+
return [
405+
` what this run did NOT read — the line above is a bounded claim, not a clean bill:`,
406+
` · ${stats.exempt} file(s) skipped whole (EXEMPT_FILES: D6/D6b/D6c and foreign vocabularies)`,
407+
` · ${stats.optOuts} literal(s) suppressed by an adr0112-ok: reason`,
408+
` · ${known.length} known lowercase code(s) deferred to their owning card (KNOWN_LOWERCASE_CODES)`,
409+
` · a code value with NO literal at the position — a constant, a template, a ternary,`,
410+
` a helper parameter — is out of reach for every pattern here by construction; that`,
411+
` half belongs to check:dispatcher-error-vocabulary, which reports its own scope.`,
412+
].join('\n');
413+
}
414+
245415
// Exports bindings, so an import for those exports alone must run nothing (#10667).
246416
if (isEntrypoint(import.meta.url)) {
247417
main();

0 commit comments

Comments
 (0)