feat(ci): check:test-masking flags inline-reimplemented prod conditions (#6348)

[diegosouzapw]

Extends check:test-masking (v2, 6A.10) to catch the wrong-shape contract test — a test that recomputes the condition under test inline instead of importing/exercising the real function. Live precedent: PR #6216 (=== 500 → >= 500) stayed green because its tests re-implemented the branch inline; the regression only surfaced one CI round later.

What changed (scripts/check/check-test-masking.mjs)

• New report-only subcheck: for each added/modified test file, flag when it (1) textually duplicates a ≥3-token conditional from a production file changed in the same PR AND (2) does not import the symbol/module owning that condition.
• Pure, fixture-testable findReimplementedConditions() + an allowlist mirroring assertReductionAllowlist.
• Report-only (warns, does not fail the gate) — to be promoted to blocking after a triage cycle, per the ratchet playbook.

Tests (Hard Rule #18)

tests/unit/check-test-masking.test.ts (45) — masked case flagged; imported-and-exercised case + <3-token trivial guard not flagged.

Closes #6348

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[gemini-code-assist]

Code Review

This pull request introduces a new report-only subcheck to detect tests that inline-reimplement production conditions instead of importing their owning symbols, along with comprehensive unit tests. The reviewer provided highly constructive feedback to improve the heuristic's robustness, including: updating the declaration regex to support TypeScript type annotations and class declarations, grouping extracted conditions to prevent false positives when a condition is shared across multiple production files, and ensuring parentheses are properly balanced before extracting if conditions to avoid parsing errors.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

In TypeScript, variable declarations often have type annotations (e.g., const isServerError: Handler = ...) and classes are frequently used. The current declRe fails to capture variable names with type annotations because of the strict \s*= check, and it completely ignores class declarations. This leads to false positives where conditions are flagged as reimplemented even when their owning symbols or classes are imported.

We can make the type annotation optional in the regex and add support for class declarations to ensure they are correctly resolved as owners.

Suggested change

	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 declRe =
	/(?:export\s+)?(?:default\s+)?(?:async\s+)?function\s+([A-Za-z_$][\w$]*)|(?:export\s+)?(?:const|let|var)\s+([A-Za-z_$][\w$]*)(?:\s*:[^=]+)?\s*=|(?:\bclass\s+([A-Za-z_$][\w$]*))/g;
	let dm;
	while ((dm = declRe.exec(src))) {
	decls.push({ index: dm.index, name: dm[1] || dm[2] || dm[3] });
	}

[gemini-code-assist]

If a common condition (such as status >= 500) is used in multiple changed production files, and the test imports the owner from one of them but not the other, the current implementation will still flag the condition as reimplemented.

To prevent this false positive, we should group all extracted conditions from all production files and map them to their set of owners. If the test imports at least one of the owners of that condition, we can safely exempt it.

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 condOwners = new Map();
  for (const prod of prodSources || []) {
    for (const { condition, owner } of extractProdConditions(prod)) {
      const sq = squash(condition);
      if (!condOwners.has(sq)) {
        condOwners.set(sq, { original: condition, owners: new Set() });
      }
      if (owner) {
        condOwners.get(sq).owners.add(owner);
      }
    }
  }

  for (const [sq, { original, owners }] of condOwners) {
    let importedAnyOwner = false;
    for (const owner of owners) {
      if (imports.has(owner)) {
        importedAnyOwner = true;
        break;
      }
    }
    if (importedAnyOwner) continue;

    if (testSq.includes(sq)) {
      const owner = owners.size > 0 ? Array.from(owners)[0] : null;
      flags.push({ condition: original, owner });
    }
  }
  return flags;
}

[gemini-code-assist]

If the parentheses in an if statement are unbalanced (e.g., due to syntax errors, or strings/comments containing parentheses), depth will remain greater than 0 when the loop terminates. In this case, pushCond is called with a potentially huge slice of code extending to the end of the file, which can cause false matches or performance issues. We should only extract the condition if the parentheses were successfully balanced (depth === 0).

Suggested change

	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));
	}
	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--;
	}
	if (depth === 0) {
	pushCond(src.slice(m.index + m[0].length, i - 1), ownerAt(m.index));
	}
	}