From e32557e4be563bbd4e1d052a4f3e91b66affd176 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Mon, 6 Jul 2026 16:03:19 -0300 Subject: [PATCH] feat(ci): check:test-masking flags inline-reimplemented prod conditions (#6348) New report-only subcheck: for each added/modified test file, warn when it textually duplicates a >=3-token conditional from a production file touched in the same PR AND does not import the symbol/module owning it (the #6216 wrong-shape-contract-test class). Pure findReimplementedConditions() + allowlist mirroring assertReductionAllowlist. Report-only for now. Closes #6348 --- CHANGELOG.md | 2 + scripts/check/check-test-masking.mjs | 210 ++++++++++++++++++++++++++ tests/unit/check-test-masking.test.ts | 131 ++++++++++++++++ 3 files changed, 343 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62b1d7ae6e2..2f06b2a7a23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ### ✨ New Features +- **feat(ci):** `check:test-masking` now flags **inline-reimplemented prod conditions** ([#6348](https://github.com/diegosouzapw/OmniRoute/issues/6348)) — a new **report-only** subcheck (v2, 6A.10 family) catches the wrong-shape contract test: a test that recomputes the condition under test inline instead of importing/exercising the real function (the #6216 class, where `=== 500` → `>= 500` stayed green because the test re-implemented the branch). For each added/modified test file it warns when the file textually duplicates a ≥3-token conditional from a production file touched in the same PR **and** does not import the symbol/module owning it, via a pure, fixture-tested `findReimplementedConditions()` with an allowlist mirroring `assertReductionAllowlist`. Report-only for now (does not fail the gate) — to be promoted to blocking after a triage cycle. Regression guard: `tests/unit/check-test-masking.test.ts` (45). + - **feat(providers):** add **Yuanbao (web)** as a cookie-session provider ([#6196](https://github.com/diegosouzapw/OmniRoute/issues/6196)) — `yuanbao-web` (Tencent Yuanbao, `yuanbao.tencent.com`) with cookie-only auth (`hy_user`/`hy_token` + public agent id), SSE→OpenAI translation incl. `reasoning_content`, exposing DeepSeek V3/R1 + Hunyuan / Hunyuan-T1. Regression guard: `tests/unit/providers-yuanbao-web.test.ts`. `together-web` was **deferred** (no verifiable web-session endpoint — needs a captured request) and `huggingchat-web` **dropped** (the existing `huggingchat` already is a web-cookie provider). (thanks @chirag127) - **feat(providers):** route the built-in **agentrouter** through the dynamic Claude-Code wire image ([#6056](https://github.com/diegosouzapw/OmniRoute/issues/6056)) — a small static allow-set (`CC_WIRE_IMAGE_BUILTINS` in `open-sse/services/ccWireImageBuiltins.ts`), consulted by `isClaudeCodeCompatible` / `isClaudeCodeCompatibleProvider` / `applyFingerprint`, makes agentrouter adopt the CC wire-image headers + fingerprint **while guarding the CC baseUrl/auth branches** so it keeps its own registry `baseUrl` and `x-api-key` auth. Regression guard: `tests/unit/agentrouter-cc-wire-image.test.ts` (asserts the wire image is applied AND agentrouter's baseUrl/auth are preserved). Live WAF-acceptance against agentrouter.org is a VPS validation follow-up (Hard Rule #18). - **feat(providers):** **bulk-add API keys for Cloudflare Workers AI** ([#6174](https://github.com/diegosouzapw/OmniRoute/issues/6174)) — `cloudflare-ai` is removed from the bulk-add exclusion list and the bulk parser gains a 3-field `name|accountId|apiKey` mode; the bulk route now builds a **per-entry** `providerSpecificData` so each key carries its own `accountId` (fixing the previous shared-object reuse), and both the create + key-validation paths receive it. Regression guard: `tests/unit/bulk-api-key-parser-cloudflare.test.ts`. (thanks @muflifadla38) diff --git a/scripts/check/check-test-masking.mjs b/scripts/check/check-test-masking.mjs index 47b0060a58b..1a2dddf95ff 100644 --- a/scripts/check/check-test-masking.mjs +++ b/scripts/check/check-test-masking.mjs @@ -15,6 +15,8 @@ import { execFileSync } from "node:child_process"; import { pathToFileURL } from "node:url"; const TEST_RE = /\.(test|spec)\.(ts|tsx)$/; +// Production TypeScript sources (excludes test files, handled separately via TEST_RE). +const PROD_SRC_RE = /\.(ts|tsx|mts|cts)$/; /** Conta chamadas de assert.*( / assert( / expect( . */ export function countAssertions(src) { @@ -57,6 +59,169 @@ export function countExtendedTautologies(src) { return count; } +// ─── (6348) Subcheck 4: inline-reimplemented prod conditions (REPORT-ONLY) ─── +// A test that copies a conditional expression out of production code (instead of +// importing and exercising the symbol that owns it) is the wrong-shape-contract-test +// class (#6216): the assertion re-encodes the branch locally, so it stays green even +// when the real prod condition drifts. This subcheck is a HEURISTIC, textual gate +// mirroring the count* helpers above — it never parses an AST. It is REPORT-ONLY for +// now (warns, does not fail the gate). + +/** Collapse all runs of whitespace to a single space and trim. */ +function normalizeWhitespace(s) { + return (s || "").replace(/\s+/g, " ").trim(); +} + +/** + * Count "significant" tokens in a normalized condition. Single-char identifiers + * (`x`, `i`) and single-digit numeric literals (`0`, `1`) are treated as noise and + * NOT counted; operators and multi-char identifiers/numbers ARE. This is what makes + * `x > 0` trivial (1 significant token) while `status >= 500` is meaningful (3): + * - `status >= 500` → status, >=, 500 → 3 + * - `x === LIMIT && y` → ===, LIMIT, && → 3 + * - `x > 0` → > → 1 + */ +export function countSignificantTokens(cond) { + const tokens = + (cond || "").match( + /===|!==|==|!=|>=|<=|&&|\|\||[<>+\-*/%!]|[A-Za-z_$][\w$]*|\d+(?:\.\d+)?/g + ) || []; + let count = 0; + for (const tk of tokens) { + if (/^[A-Za-z_$]/.test(tk)) { + if (tk.length >= 2) count++; // multi-char identifier + } else if (/^\d/.test(tk)) { + if (tk.length >= 2) count++; // multi-digit number + } else { + count++; // operator + } + } + return count; +} + +/** A condition is "meaningful" when it carries ≥3 significant tokens. */ +function isSignificantCondition(cond) { + return countSignificantTokens(cond) >= 3; +} + +/** + * Extract meaningful (≥3-token) conditional expressions from a production source, + * paired with the nearest enclosing declared symbol that "owns" them. Covers + * `if (...)` (via paren balancing) and comparison-bearing ternaries (`a === b ? … : …`). + * Returns [{ condition (whitespace-normalized), owner }]. + */ +export function extractProdConditions(src) { + const results = []; + if (!src) return results; + + // Declarations (function / const / let / var) with their positions, so each + // condition can be attributed to the symbol whose body it lives in. + const decls = []; + const declRe = + /(?:export\s+)?(?:default\s+)?(?:async\s+)?function\s+([A-Za-z_$][\w$]*)|(?:export\s+)?(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=/g; + let dm; + while ((dm = declRe.exec(src))) { + decls.push({ index: dm.index, name: dm[1] || dm[2] }); + } + const ownerAt = (idx) => { + let owner = ""; + for (const d of decls) { + if (d.index <= idx) owner = d.name; + else break; + } + return owner; + }; + + const seen = new Set(); + const pushCond = (raw, owner) => { + const norm = normalizeWhitespace(raw); + if (!norm || seen.has(norm) || !isSignificantCondition(norm)) return; + seen.add(norm); + results.push({ condition: norm, owner }); + }; + + // if (...) — balance parentheses to capture the full condition. + const ifRe = /\bif\s*\(/g; + let m; + while ((m = ifRe.exec(src))) { + let depth = 1; + let i = m.index + m[0].length; + for (; i < src.length && depth > 0; i++) { + const ch = src[i]; + if (ch === "(") depth++; + else if (ch === ")") depth--; + } + pushCond(src.slice(m.index + m[0].length, i - 1), ownerAt(m.index)); + } + + // Comparison-bearing ternaries: ` ? … : …` (best-effort, low-noise). + const ternRe = + /([A-Za-z_$][\w$).\]]*\s*(?:===|!==|==|!=|>=|<=|>|<)\s*[^?;{}\n]+?)\s*\?/g; + let t; + while ((t = ternRe.exec(src))) { + pushCond(t[1], ownerAt(t.index)); + } + + return results; +} + +/** + * Collect the identifiers/module specifiers a test file imports, so we can tell + * whether it exercises a prod symbol through the real import (clean) or merely + * re-implements one of its conditions locally (masked). Returns a Set of names: + * imported bindings, module paths, and module basenames. + */ +export function extractImports(src) { + const names = new Set(); + if (!src) return names; + const addModule = (mod) => { + names.add(mod); + const base = mod.split("/").pop().replace(/\.\w+$/, ""); + if (base) names.add(base); + }; + let m; + const importRe = /import\s+(?:type\s+)?([^;]*?)\s+from\s+['"]([^'"]+)['"]/g; + while ((m = importRe.exec(src))) { + addModule(m[2]); + for (const id of m[1].match(/[A-Za-z_$][\w$]*/g) || []) { + if (id !== "as" && id !== "type") names.add(id); + } + } + const dynRe = /import\s*\(\s*['"]([^'"]+)['"]\s*\)/g; + while ((m = dynRe.exec(src))) addModule(m[1]); + const reqRe = /require\s*\(\s*['"]([^'"]+)['"]\s*\)/g; + while ((m = reqRe.exec(src))) addModule(m[1]); + return names; +} + +/** + * PURE core of subcheck 4. Given the sources of the prod files changed in a PR, + * one test file's source, and the set of names that test imports: return the prod + * conditions the test re-implements textually WITHOUT importing the symbol that owns + * them. Whitespace is squashed on both sides so spacing differences never mask a hit. + * Returns [{ condition, owner }]. + */ +export function findReimplementedConditions(prodSources, testSource, testImports) { + const flags = []; + if (!testSource) return flags; + const imports = + testImports instanceof Set ? testImports : new Set(testImports || []); + const squash = (s) => (s || "").replace(/\s+/g, ""); + const testSq = squash(testSource); + const seen = new Set(); + for (const prod of prodSources || []) { + for (const { condition, owner } of extractProdConditions(prod)) { + if (owner && imports.has(owner)) continue; // exercised through the real import + if (seen.has(condition)) continue; + if (testSq.includes(squash(condition))) { + seen.add(condition); + flags.push({ condition, owner: owner || null }); + } + } + } + return flags; +} + /** * (6A.10 subcheck 1) Sinaliza arquivos de teste DELETADOS ou renomeados-e-não- * substituídos. Recebe lista de paths de arquivos de teste que foram deletados @@ -241,14 +406,59 @@ function main() { // Only exempts the reduction signal; tautology/skip/deletion signals still fire. let assertReductionAllowlist = new Set(); let deletionAllowlist = {}; + let reimplementedAllowlist = new Set(); try { const raw = JSON.parse(fs.readFileSync("config/quality/test-masking-allowlist.json", "utf8")); assertReductionAllowlist = new Set(Object.keys(raw).filter((k) => !k.startsWith("_"))); deletionAllowlist = raw._deletedWithReplacement || {}; + reimplementedAllowlist = new Set(raw._reimplementedConditions || []); } catch { // no allowlist file — treat as empty } + // (6348 subcheck 4, REPORT-ONLY) Tests that inline-reimplement a prod condition + // instead of importing the symbol that owns it. Prod files changed in this PR + // (added/copied/modified TS sources) are the reference corpus; each changed test + // file is scanned against them. Warns only — it never fails the gate for now. + const prodChanged = git(["diff", "--name-only", "--diff-filter=ACM", `${base}...HEAD`]) + .split("\n") + .map((s) => s.trim()) + .filter((f) => PROD_SRC_RE.test(f) && !TEST_RE.test(f) && fs.existsSync(f)); + const prodSources = prodChanged.map((f) => { + try { + return fs.readFileSync(f, "utf8"); + } catch { + return ""; + } + }); + const changedTests = git(["diff", "--name-only", "--diff-filter=ACM", `${base}...HEAD`]) + .split("\n") + .map((s) => s.trim()) + .filter((f) => TEST_RE.test(f) && fs.existsSync(f)); + const reimplementedFlags = []; + if (prodSources.length) { + for (const tf of changedTests) { + if (reimplementedAllowlist.has(tf)) continue; + const src = fs.readFileSync(tf, "utf8"); + for (const hit of findReimplementedConditions(prodSources, src, extractImports(src))) { + reimplementedFlags.push( + `${tf}: re-implementa a condição \`${hit.condition}\`` + + (hit.owner ? ` (dona: ${hit.owner})` : "") + + " — asserte através do import real em vez de copiar a condição" + ); + } + } + } + if (reimplementedFlags.length) { + console.warn( + `[test-masking] (report-only) ${reimplementedFlags.length} teste(s) re-implementam ` + + `condição de produção em vez de importar o símbolo dono (classe #6216):\n` + + reimplementedFlags.map((f) => " ⚠ " + f).join("\n") + + `\n → importe o símbolo/função dono e asserte através dele (evita contrato duplicado ` + + `que diverge silenciosamente). Report-only por enquanto — não falha o gate.` + ); + } + const deletedFlags = evaluateDeletedFiles( [...deletedTests, ...relocatedOutOfTest], deletionAllowlist diff --git a/tests/unit/check-test-masking.test.ts b/tests/unit/check-test-masking.test.ts index 92d9d713b14..662b44c792b 100644 --- a/tests/unit/check-test-masking.test.ts +++ b/tests/unit/check-test-masking.test.ts @@ -8,6 +8,10 @@ import { evaluateMasking, evaluateDeletedFiles, partitionDeletedRenamed, + countSignificantTokens, + extractProdConditions, + extractImports, + findReimplementedConditions, } from "../../scripts/check/check-test-masking.mjs"; // ─── Existing tests (must stay green) ──────────────────────────────────────── @@ -409,3 +413,130 @@ test("evaluateMasking: allowlist exempts ONLY reduction — tautology/skip still assert.ok(r.some((f) => /tautolog/i.test(f))); assert.ok(r.some((f) => /skip/i.test(f))); }); + +// ─── 6348 Subcheck 4: inline-reimplemented prod conditions (REPORT-ONLY) ────── + +test("countSignificantTokens: `status >= 500` has 3 significant tokens", () => { + assert.equal(countSignificantTokens("status >= 500"), 3); +}); + +test("countSignificantTokens: `x === LIMIT && y` has 3 significant tokens", () => { + assert.equal(countSignificantTokens("x === LIMIT && y"), 3); +}); + +test("countSignificantTokens: trivial `x > 0` has <3 significant tokens (noise guard)", () => { + assert.ok(countSignificantTokens("x > 0") < 3); +}); + +test("extractProdConditions: pulls the if-condition and its owning symbol", () => { + const prod = [ + "export function isServerError(status) {", + " if (status >= 500) {", + " return true;", + " }", + " return false;", + "}", + ].join("\n"); + const conds = extractProdConditions(prod); + const hit = conds.find((c) => c.condition === "status >= 500"); + assert.ok(hit, "the ≥3-token condition is extracted"); + assert.equal(hit.owner, "isServerError"); +}); + +test("extractProdConditions: ignores trivial <3-token conditions", () => { + const prod = "export function f(x) {\n if (x > 0) return 1;\n return 0;\n}"; + assert.deepEqual(extractProdConditions(prod), []); +}); + +test("extractImports: collects named bindings and module specifiers", () => { + const src = `import { isServerError, foo } from "../src/http";\nimport def from "./bar";`; + const names = extractImports(src); + assert.ok(names.has("isServerError")); + assert.ok(names.has("foo")); + assert.ok(names.has("def")); +}); + +const PROD_SERVER_ERROR = [ + "export function isServerError(status) {", + " if (status >= 500) {", + " return true;", + " }", + " return false;", + "}", +].join("\n"); + +test("MASKED: test re-implements `status >= 500` without importing the owner → flagged", () => { + const testSrc = [ + 'import { test } from "node:test";', + 'import assert from "node:assert";', + "// re-encodes the prod branch locally instead of importing isServerError", + "function localCheck(status) {", + " return status >= 500;", + "}", + 'test("server error", () => {', + " assert.equal(localCheck(503), true);", + "});", + ].join("\n"); + const flags = findReimplementedConditions( + [PROD_SERVER_ERROR], + testSrc, + extractImports(testSrc) + ); + assert.equal(flags.length, 1); + assert.equal(flags[0].condition, "status >= 500"); + assert.equal(flags[0].owner, "isServerError"); +}); + +test("CLEAN: test imports and calls the real function → not flagged", () => { + const testSrc = [ + 'import { test } from "node:test";', + 'import assert from "node:assert";', + 'import { isServerError } from "../../src/http";', + 'test("server error", () => {', + " assert.equal(isServerError(503), true);", + " assert.equal(isServerError(200), false);", + "});", + ].join("\n"); + const flags = findReimplementedConditions( + [PROD_SERVER_ERROR], + testSrc, + extractImports(testSrc) + ); + assert.deepEqual(flags, []); +}); + +test("CLEAN: importing the owner exempts even a textual copy of its condition", () => { + // The test both imports the owner AND happens to spell the condition — importing + // the owner means it exercises the real symbol, so the condition is not masked. + const testSrc = [ + 'import { isServerError } from "../../src/http";', + "// documents that isServerError fires when status >= 500", + 'test("t", () => { isServerError(503); });', + ].join("\n"); + const flags = findReimplementedConditions( + [PROD_SERVER_ERROR], + testSrc, + extractImports(testSrc) + ); + assert.deepEqual(flags, []); +}); + +test("CLEAN: trivial `x > 0` condition is never flagged (noise guard)", () => { + const prod = "export function f(x) {\n if (x > 0) return 1;\n return 0;\n}"; + const testSrc = [ + 'import { test } from "node:test";', + "function local(x) { return x > 0; }", + 'test("t", () => { local(1); });', + ].join("\n"); + const flags = findReimplementedConditions([prod], testSrc, extractImports(testSrc)); + assert.deepEqual(flags, []); +}); + +test("findReimplementedConditions: matches despite operator-spacing differences", () => { + const prod = "export function big(status) {\n if (status >= 500) return true;\n}"; + // test spells the same condition with no spaces around the operator + const testSrc = "function local(status){ return status>=500; }"; + const flags = findReimplementedConditions([prod], testSrc, extractImports(testSrc)); + assert.equal(flags.length, 1); + assert.equal(flags[0].condition, "status >= 500"); +});