mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
check:test-masking flags inline-reimplemented prod (#6348) (net +1/-0, tests OK). Integrated into release/v3.8.46.
This commit is contained in:
committed by
GitHub
parent
c7c7d476a3
commit
ecf3d3aa06
@@ -8,6 +8,7 @@
|
||||
|
||||
### ✨ 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(sse):** per-connection routing override (native vs CLIProxyAPI) ([#6339](https://github.com/diegosouzapw/OmniRoute/issues/6339)) — the previously-dead `isCliproxyapiDeepModeEnabled` helper is now wired into `resolveExecutorWithProxy`: a single connection can opt itself into the CLIProxyAPI passthrough executor via `providerSpecificData.cliproxyapiMode="claude-native"`, with precedence **connection override > provider `upstream_proxy_config` mode > default**. `resolveExecutorWithProxy` now receives the resolved connection's `providerSpecificData` (threaded from `chatCore.ts`), so one connection can deep-route while the provider's other connections stay native — no DB schema change (the toggle rides in `providerSpecificData`). Also resolves the same-provider mixing ask in #6340. Regression guard: `tests/unit/chatcore-executor-proxy.test.ts` (9). (thanks @RaviTharuma)
|
||||
- **feat(dashboard):** "Add session cookie" modal now shows a prominent **"Open ‹host› →"** link to the provider's own site ([#6268](https://github.com/diegosouzapw/OmniRoute/issues/6268)) — every `-web` cookie-session provider (chatgpt-web, claude-web, gemini-web, kimi-web, lmarena, qwen-web, m365-copilot-web, …) renders a one-click external link (opening the provider's login/home page in a new tab) so operators no longer tab away to retype the URL mid-setup. The host resolves from a pure, unit-tested `resolveWebProviderHost()` (prefers `WEB_COOKIE_PROVIDERS[id].website`, falls back to the registry `baseUrl` origin); non-web providers render exactly as before. Kilo's dup flag vs #6265 (modal-too-small-on-1080p) was a false positive — distinct concern. Regression guard: `tests/unit/resolve-web-provider-host.test.ts` (5). (thanks @chirag127)
|
||||
- **feat(providers):** add **DigitalOcean** AI (serverless inference) as an OpenAI-compatible API-key provider — base `https://inference.do-ai.run/v1`, wired through the shared OpenAI-compatible registry with full model passthrough (`open-sse/config/providers/registry/digitalocean/`, `src/shared/constants/providers/apikey/inference-hosts.ts`). Regression guard: `tests/unit/digitalocean-provider.test.ts`. (thanks @newnol)
|
||||
|
||||
@@ -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: `<lhs> <cmp> <rhs> ? … : …` (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
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user