diff --git a/internal/check-no-silent-engine-substitution.mjs b/internal/check-no-silent-engine-substitution.mjs index 73ebf951c..9bdc45a99 100644 --- a/internal/check-no-silent-engine-substitution.mjs +++ b/internal/check-no-silent-engine-substitution.mjs @@ -63,12 +63,36 @@ const PATTERN = new RegExp( function scan(text) { const hits = [] const lines = text.split('\n') + // ⚠ BLOCK-COMMENT STATE, not a per-line prefix test. The first version asked only whether a line + // STARTS with //, * or /*, and that is not what a comment is: it misses every CONTINUATION line of a + // /* … */ block whose text happens to begin with something else. + // + // IT COST A FALSE ACCUSATION, 2026-09-27, and the accusation was aimed at the correct fix. gospel + // fixed this defect in resources-map.vue — `const gl = engineGlobal(grProvider)`, no `||` — and + // documented what they removed in a /* */ block whose second line begins with a BACKTICK: + // `engineGlobal(grProvider) || window.mapboxgl`. engineGlobal already defaults to mapbox, so + // The prefix test saw no comment marker, matched the quoted pattern, and reported gospel as the last + // remaining offender at main 9df086e5. I relayed that to dx-shepherd as fact and it was wrong: the + // line number had also moved three times (1490 -> 1505 -> 1509) purely because their file grew, which + // is what made a stale card look like a live defect. + // ★ A GATE THAT FAILS ON THE EXPLANATION OF A FIX IS WORSE THAN NO GATE. It tells the one person who + // did the right thing that they did the wrong thing, and the cheapest way to make it green is to + // DELETE the explanation — so the gate actively destroys the reasoning it should protect. This file's + // own header quotes the pattern for the same reason, which is why the skip existed at all; it was + // just too shallow to cover the form another agent actually used. + let inBlock = false lines.forEach((line, i) => { - // Skip comment lines: this file's own documentation quotes the bad pattern, and so does every fix - // that explains what it removed. A gate that fails on its own explanation teaches people to delete - // the explanation. const t = line.trim() - if (t.startsWith('//') || t.startsWith('*') || t.startsWith('/*')) return + const wasInBlock = inBlock + // Track /* and */ on this line so the NEXT line knows where it stands. Deliberately simple: a + // block marker inside a string literal would fool it, and the alternative is a JS parser for a + // one-expression grep. The failure mode of over-skipping is a missed offender, which the + // --self-test guards against by keeping real forms in the MUST-MATCH list. + const opens = (line.match(/\/\*/g) || []).length + const closes = (line.match(/\*\//g) || []).length + if (opens > closes) inBlock = true + else if (closes > opens) inBlock = false + if (wasInBlock || t.startsWith('//') || t.startsWith('*') || t.startsWith('/*')) return PATTERN.lastIndex = 0 let m while ((m = PATTERN.exec(line)) !== null) hits.push({ line: i + 1, engine: m[1], text: t.slice(0, 110) }) @@ -90,6 +114,21 @@ if (SELF_TEST) { 'const gl = engineGlobal(p) || null', 'const token = mapboxToken?.value || profileConfig?.value?.tk || \'\'', ] + // ★ MULTI-LINE CASES, because the single-line list above cannot express the bug that actually + // happened: a CONTINUATION line of a /* */ block. Each entry is scanned as one text so the + // block-comment state machine is what is under test, not the prefix check. + const shouldNotMultiline = [ + // gospel's real fix at resources-map.vue, verbatim in shape — the line that was falsely accused + // begins with a BACKTICK, so no prefix test could ever have skipped it. + [` /* ⚠ NEVER SWITCH ENGINES SILENTLY. This used to read + \`engineGlobal(grProvider) || window.mapboxgl\`. engineGlobal already defaults to mapbox, so + the fallback could only ever fire for provider 'maplibre'. */ + const gl = engineGlobal(grProvider)`, 'a /* */ block quoting the removed fallback, then the correct fix'], + // and the inverse: a real offender AFTER a block comment closes must still be caught, or the fix + // above would have traded a false positive for a false negative. + [` /* explanation that mentions engineGlobal(p) || window.mapboxgl and then ends */ + const gl = engineGlobal(p) || window.mapboxgl`, null], + ] let bad = 0 for (const s of should) { const n = scan(s).length @@ -101,6 +140,17 @@ if (SELF_TEST) { console.log(` ${n ? 'FALSE+ ' : 'ignored '} ${s.trim().slice(0, 62)}`) if (n) bad++ } + // The multi-line pairs: a label means "must be IGNORED and here is why", null means "must MATCH". + // Both directions are asserted together on purpose — fixing the false positive by skipping more is + // only correct if a real offender after the block still fails. + for (const [text, why] of shouldNotMultiline) { + const n = scan(text).length + const wantHit = why === null + const ok = wantHit ? n > 0 : n === 0 + if (!ok) bad++ + console.log(` ${ok ? (wantHit ? 'MATCHED ' : 'ignored ') : (wantHit ? 'MISSED ' : 'FALSE+ ')} ` + + `[multi-line] ${wantHit ? 'a real offender AFTER a closed block comment' : why}`) + } if (bad) { console.error(`\nNO-SILENT-ENGINE-SUBSTITUTION --self-test: FAIL — ${bad} case(s) wrong.`) process.exit(1)