fix: repair stray brace in combo.ts and fix no-explicit-any types in repro-9630 test

Refs #9673

.env.example

@@ -2483,3 +2483,8 @@ QUOTA_STORE_DRIVER=sqlite              # sqlite | redis
 # signature, replacing the pinned default key. Required when self-hosting a
 # feed signed with a different key pair.
 # RADAR_FEED_PUBKEY=
+
+# Threshold for the forgotten-sibling-tests quality gate. When the number of
+# changed source files in a PR exceeds this value, the consumer scan is skipped
+# (typically for release PRs or mass refactors). Default is 300.
+# FORGOTTEN_SIBLING_MAX_CHANGED=300

.github/workflows/ci.yml

@@ -587,6 +587,9 @@ jobs:
       # Anti test-masking: flag net assert removal / new assert.ok(true) in changed tests.
       - name: Detect test-masking (weakened assertions)
         run: npm run check:test-masking
+      # Detect forgotten sibling tests (consumers whose test sibling is not in the diff).
+      - name: Detect forgotten sibling tests
+        run: npm run check:forgotten-sibling-tests
       # Evidence-in-PR-body (Hard Rule #18 mechanized): claims of "tests pass" must carry output.
       - name: Require evidence in PR body
         run: npm run check:pr-evidence

changelog.d/features/9530-forgotten-sibling-tests-gate.md

@@ -0,0 +1 @@
+- **feat(ci):** new quality gate `check-forgotten-sibling-tests` detects when a source symbol changes but consumer tests are not in the same diff — preventing the 7 documented "forgotten sibling test" occurrences from PR #9529 ([#9530](https://github.com/diegosouzapw/OmniRoute/issues/9530))

changelog.d/fixes/9632-connection-test-network-error-status.md

@@ -1 +0,0 @@
-- **fix(api):** keep the stored connection `testStatus` when a test never reaches the upstream — a `network_error` diagnosis (request timed out locally or aborted) no longer overwrites the stored status; the error fields are still recorded so the attempt is visible. Also fixes a second gap where `classifyFailure` matched `"timeout"` as a substring but `testOAuthConnection` reports its own abort as `Test timed out after 30s` (no `"timeout"` in that string), so an OAuth probe that hit the 30s ceiling was misclassified as `upstream_error` rather than `network_error`. ([#9623](https://github.com/diegosouzapw/OmniRoute/issues/9623)) — thanks @HouMinXi

config/quality/forgotten-sibling-allowlist.json

@@ -0,0 +1,10 @@
+{
+  "_comment": "Forgotten-sibling-tests allowlist (check-forgotten-sibling-tests.mjs). Each entry exempts a (sourcePath, forgottenSibling) pair. Use when a consumer test legitimately does not need changes (compatible refactor, same interface). Every entry needs a reason with the tracking issue or PR ref. Wildcard sourcePath=\"*\" exempts the sibling regardless of which source triggered it.",
+  "_schema": [
+    {
+      "sourcePath": "src/lib/example.ts",
+      "forgottenSibling": "tests/unit/lib/example-consumer.test.ts",
+      "reason": "Issue #1234: compatible interface change only — consumer tests verify contract, not implementation"
+    }
+  ]
+}

docs/architecture/QUALITY_GATES.md

@@ -176,11 +176,12 @@ Full i18n validation matrix (one job per locale). Entire job is advisory.
 
 Runs on pull requests only.
 
-| Script                 | Validates                                                                                                                  | Blocking |
-| ---------------------- | -------------------------------------------------------------------------------------------------------------------------- | -------- |
-| `check:pr-test-policy` | PRs that change production code in `src/`, `open-sse/`, `electron/`, or `bin/` must include or update tests (Hard Rule #8) | Yes      |
-| `check:test-masking`   | Changed test files do not reduce net assert count or add `assert.ok(true)` tautologies                                     | Yes      |
-| `check:pr-evidence`    | PR body cites test/VPS evidence for the change (mechanizes Hard Rule #18 by grepping PR prose — fragile, see Backlog)      | Yes      |
+| Script                          | Validates                                                                                                                                                                           | Blocking |
+| ------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- |
+| `check:pr-test-policy`          | PRs that change production code in `src/`, `open-sse/`, `electron/`, or `bin/` must include or update tests (Hard Rule #8)                                                          | Yes      |
+| `check:test-masking`            | Changed test files do not reduce net assert count or add `assert.ok(true)` tautologies                                                                                              | Yes      |
+| `check:pr-evidence`             | PR body cites test/VPS evidence for the change (mechanizes Hard Rule #18 by grepping PR prose — fragile, see Backlog)                                                               | Yes      |
+| `check:forgotten-sibling-tests` | When a PR changes source file Y, detects if any production consumer Z of Y has a test sibling that is NOT included in the same diff — preventing the forgotten-sibling-test pattern | Yes      |
 
 ### Job: `test-vitest`
 

docs/reference/ENVIRONMENT.md

@@ -1353,6 +1353,12 @@ Used by `src/lib/vncSession/manifest.ts` to configure Docker-based headless Chro
 | `OMNIROUTE_VNC_HARVEST_MS`            | `20000`                       | `src/lib/vncSession/manifest.ts`  | Harvest/cleanup timeout (ms).                                                  |
 | `VIBEPROXY_DATA_DIR`                  | _(unset)_                     | `open-sse/services/notionThreadSessions.ts` | Directory for Notion thread session persistence.                               |
 
+### Quality gate scripts (CI)
+
+| Variable | Default | Description |
+| --- | --- | --- |
+| `FORGOTTEN_SIBLING_MAX_CHANGED` | `300` | Threshold for the forgotten-sibling-tests quality gate. When a PR changes more source files than this value, the consumer dependency scan is skipped (release PR or mass refactor). |
+
 ### Internal service auth
 
 | Variable | Default | Description |

open-sse/services/combo.ts

@@ -2065,7 +2065,6 @@ export async function handleComboChat({
 	          { code: "ALL_ACCOUNTS_INACTIVE", type: "service_unavailable" }
 	        );
 	      }
-      }
 
       const status = lastStatus;
       // Build aggregated error message with per-model failure details for diagnostics.

package.json

@@ -230,6 +230,7 @@
     "coverage:report": "cross-env NODE_OPTIONS=--max-old-space-size=8192 c8 report --merge-async --output-dir coverage --exclude=tests/** --exclude=**/*.test.* --reporter=text --reporter=text-summary --reporter=html --reporter=json-summary --reporter=lcov",
     "coverage:summary": "node scripts/check/test-report-summary.mjs --input coverage/coverage-summary.json --output coverage/coverage-report.md",
     "check:pr-test-policy": "node scripts/check/check-pr-test-policy.mjs",
+    "check:forgotten-sibling-tests": "node scripts/check/check-forgotten-sibling-tests.mjs",
     "coverage:report:legacy": "c8 report --output-dir coverage --exclude=open-sse --reporter=text --reporter=text-summary",
     "test:all": "npm run test:unit && npm run test:vitest && npm run test:vitest:ui && npm run test:ecosystem && npm run test:e2e",
     "check": "npm run lint && npm run test",

scripts/check/check-forgotten-sibling-tests.mjs

@@ -0,0 +1,241 @@
+#!/usr/bin/env node
+// scripts/check/check-forgotten-sibling-tests.mjs
+// Gate: when a PR changes source file Y, detects if any production consumer Z of Y
+// has a test sibling (Z.test.ts or Z/index.ts → Z/test.ts) that is NOT included in
+// the same PR diff. Prevents the "forgotten sibling test" pattern (7 occurrences
+// fixed in PR #9529).
+//
+// Usage:
+//   node scripts/check/check-forgotten-sibling-tests.mjs
+//
+// Environment (PR context):
+//   GITHUB_BASE_SHA or GITHUB_BASE_REF  — base of the PR diff
+//   FORGOTTEN_SIBLING_MAX_CHANGED        — threshold for release-PR skip (default 300)
+//
+// No PR context → no-ops (exit 0, no output).
+
+import fs from "node:fs";
+import path from "node:path";
+import { execFileSync } from "node:child_process";
+import { pathToFileURL } from "node:url";
+import { globSync } from "tinyglobby";
+import { ROOT, IMPORT_RE, EXTS, SRC_ROOTS, resolveImport } from "./lib/importResolution.mjs";
+
+// ─── Constants ────────────────────────────────────────────────────────────────
+
+const SOURCE_ROOTS = ["src/", "open-sse/", "bin/"];
+const EXCLUDED_PATTERNS = [
+  /\/tests\//,
+  /\/migrations\//,
+  /\/__tests__\//,
+  /\.test\./,
+  /\.spec\./,
+  /\/node_modules\//,
+  /\/config\/(?:quality|eslint|tsconfig)/,
+];
+const DEFAULT_MAX_CHANGED = 300;
+
+// ─── Helpers (exported for testing) ───────────────────────────────────────────
+
+export function runGit(args) {
+  return execFileSync("git", args, { encoding: "utf8" }).trim();
+}
+
+export function isSourceFile(filePath) {
+  if (EXCLUDED_PATTERNS.some((p) => p.test(filePath))) return false;
+  return (
+    SOURCE_ROOTS.some((root) => filePath.startsWith(root)) && EXTS.some((e) => filePath.endsWith(e))
+  );
+}
+
+export function resolveBase() {
+  if (process.env.GITHUB_BASE_SHA) return process.env.GITHUB_BASE_SHA;
+  if (process.env.GITHUB_BASE_REF) return `origin/${process.env.GITHUB_BASE_REF}`;
+  return null;
+}
+
+/**
+ * Test sibling of a production file Z.
+ * Convention: Z.ts → Z.test.ts, or Z/index.ts → Z/test.ts.
+ * Returns the repo-relative path of the test sibling, or null if none.
+ */
+export function testSiblingOf(relPath) {
+  const abs = path.join(ROOT, relPath);
+  const dir = path.dirname(abs);
+  const base = path.basename(abs).replace(/\.(ts|tsx|mts|js|mjs)$/, "");
+  const candidates = [
+    path.join(dir, `${base}.test.ts`),
+    path.join(dir, `${base}.test.tsx`),
+    path.join(dir, `${base}.test.mjs`),
+  ];
+  // Also try Z/test/ subdirectory
+  const testDir = path.join(dir.replace(/\/?$/, ""), "test");
+  candidates.push(
+    path.join(testDir, `${base}.test.ts`),
+    path.join(testDir, `${base}.test.tsx`),
+    path.join(testDir, `${base}.test.mjs`)
+  );
+  // For Z/index.ts or Z/route.ts, also try Z/test.ts
+  if (base === "index" || base === "route") {
+    candidates.push(
+      path.join(dir, `${base}.test.ts`),
+      path.join(dir, `${base}.test.tsx`),
+      path.join(dir, `${base}.test.mjs`),
+      path.join(dir, "test.ts"),
+      path.join(dir, "test.tsx"),
+      path.join(dir, "test.mjs")
+    );
+  }
+  for (const c of candidates) {
+    if (fs.existsSync(c)) return path.relative(ROOT, c);
+  }
+  return null;
+}
+
+/**
+ * For a given changed source file (repo-relative path), find all production
+ * files under SOURCE_ROOTS that directly import from it. Uses the pre-built
+ * reverse dependency map.
+ */
+export function findConsumers(changedRelPath, prodFileMap) {
+  const abs = path.join(ROOT, changedRelPath);
+  const consumers = [];
+  for (const [consumerRel, deps] of Object.entries(prodFileMap)) {
+    if (deps.has(abs)) consumers.push(consumerRel);
+  }
+  return consumers.sort();
+}
+
+/**
+ * Build a map of all production files → their resolved direct import deps (Set of absolute paths).
+ */
+export function buildProdFileMap() {
+  const map = {};
+  const prodFiles = globSync(
+    SRC_ROOTS.map((r) => `${r}/**/*.{ts,tsx,mts,js,mjs}`),
+    { cwd: ROOT, ignore: ["**/node_modules/**", "**/tests/**", "**/__tests__/**"] }
+  );
+  for (const f of prodFiles) {
+    if (!isSourceFile(f)) continue;
+    const fullPath = path.join(ROOT, f);
+    let code;
+    try {
+      code = fs.readFileSync(fullPath, "utf8");
+    } catch {
+      continue;
+    }
+    const deps = new Set();
+    for (const m of code.matchAll(IMPORT_RE)) {
+      const spec = m[1] || m[2] || m[3];
+      if (!spec) continue;
+      const r = resolveImport(spec, fullPath);
+      if (!r) continue;
+      deps.add(r);
+    }
+    map[f] = deps;
+  }
+  return map;
+}
+
+/**
+ * Check if an allowlist entry exempts a (changedFile, consumerWithMissingTest) pair.
+ */
+export function isAllowlisted(changedFile, missingTest, allowlist) {
+  for (const entry of allowlist) {
+    if (entry.sourcePath === changedFile && entry.forgottenSibling === missingTest) return true;
+    if (entry.sourcePath === "*" && entry.forgottenSibling === missingTest) return true;
+  }
+  return false;
+}
+
+// ─── Main ─────────────────────────────────────────────────────────────────────
+
+function main() {
+  const base = resolveBase();
+  if (!base) {
+    console.log("[forgotten-sibling] no base ref (not a PR context) — skipping check.");
+    return;
+  }
+
+  // Read allowlist
+  let allowlist = [];
+  try {
+    const raw = JSON.parse(
+      fs.readFileSync(path.join(ROOT, "config/quality/forgotten-sibling-allowlist.json"), "utf8")
+    );
+    allowlist = Array.isArray(raw) ? raw : [];
+  } catch {
+    // No allowlist file or parse error — treat as empty
+  }
+
+  // Get changed files
+  const changedFiles = runGit(["diff", "--name-only", "--diff-filter=ACM", `${base}...HEAD`])
+    .split(/\r?\n/)
+    .map((s) => s.trim())
+    .filter(Boolean);
+
+  const changedSources = changedFiles.filter(isSourceFile);
+  const changedTestFiles = new Set(
+    changedFiles.filter((f) => /\.(?:test|spec)\.(?:ts|tsx|mjs)$/.test(f))
+  );
+
+  // Release PR skip: if too many changed files, skip the detailed consumer walk.
+  const maxChanged = Number(process.env.FORGOTTEN_SIBLING_MAX_CHANGED) || DEFAULT_MAX_CHANGED;
+  if (maxChanged > 0 && changedSources.length > maxChanged) {
+    console.log(
+      `[forgotten-sibling] ${changedSources.length} source file(s) changed exceeds ` +
+        `threshold (${maxChanged}) — skipping consumer scan (release PR).\n` +
+        `  A diff this large is a release PR or mass refactor; each file already ` +
+        `passed this gate on its own PR during the cycle.`
+    );
+    return;
+  }
+
+  if (changedSources.length === 0) {
+    console.log("[forgotten-sibling] no changed source files — OK.");
+    return;
+  }
+
+  // Build the production dependency map (source → consumers)
+  const prodFileMap = buildProdFileMap();
+
+  const flags = [];
+
+  for (const changedSource of changedSources) {
+    const consumers = findConsumers(changedSource, prodFileMap);
+    if (consumers.length === 0) continue;
+
+    for (const consumer of consumers) {
+      const testSibling = testSiblingOf(consumer);
+      if (!testSibling) continue;
+
+      if (changedTestFiles.has(testSibling)) continue;
+
+      if (isAllowlisted(changedSource, testSibling, allowlist)) continue;
+
+      flags.push(
+        `${changedSource}: \`${consumer}\` imports this file and has a test sibling ` +
+          `(\`${testSibling}\`) that is NOT in the current diff. ` +
+          "When the public API of the changed source changes, consumer tests " +
+          "may need updating too."
+      );
+    }
+  }
+
+  if (flags.length) {
+    console.error(
+      `[forgotten-sibling] ${flags.length} forgotten sibling test(s) detected:\n` +
+        flags.map((f) => `  ✗ ${f}`).join("\n") +
+        `\n  → Add the missing test files to this PR or add an allowlist entry ` +
+        `(config/quality/forgotten-sibling-allowlist.json) with a justification.`
+    );
+    process.exit(1);
+  }
+
+  console.log(
+    `[forgotten-sibling] OK — ${changedSources.length} source file(s) changed, ` +
+      `no forgotten sibling tests.`
+  );
+}
+
+if (import.meta.url === pathToFileURL(process.argv[1]).href) main();

scripts/check/lib/importResolution.mjs

@@ -0,0 +1,67 @@
+#!/usr/bin/env node
+// scripts/check/lib/importResolution.mjs
+// Shared import resolution logic extracted from build-test-impact-map.mjs.
+// Provides resolveImport(), sourceDepsOf(), IMPORT_RE, EXTS, SRC_ROOTS, ROOT.
+
+import fs from "node:fs";
+import path from "node:path";
+import { fileURLToPath } from "node:url";
+
+export const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../../..");
+export const SRC_ROOTS = ["src", "open-sse"];
+export const IMPORT_RE =
+  /(?:import|export)[^'"]*from\s*['"]([^'"]+)['"]|require\(\s*['"]([^'"]+)['"]\s*\)|import\(\s*['"]([^'"]+)['"]\s*\)/g;
+export const EXTS = [".ts", ".tsx", ".mts", ".js", ".mjs"];
+
+/**
+ * Resolve an import specifier to an absolute file path.
+ * Handles `@/` aliases, `@omniroute/open-sse` aliases, and relative paths.
+ * Returns null for external/npm imports or unresolvable specs.
+ */
+export function resolveImport(spec, fromFile) {
+  let base;
+  if (spec.startsWith("@/")) base = path.join(ROOT, "src", spec.slice(2));
+  else if (spec.startsWith("@omniroute/open-sse"))
+    base = path.join(ROOT, "open-sse", spec.replace(/^@omniroute\/open-sse\/?/, ""));
+  else if (spec.startsWith(".")) base = path.resolve(path.dirname(fromFile), spec);
+  else return null;
+  for (const e of EXTS) {
+    if (fs.existsSync(base + e)) return base + e;
+  }
+  for (const e of EXTS) {
+    const idx = path.join(base, "index" + e);
+    if (fs.existsSync(idx)) return idx;
+  }
+  return fs.existsSync(base) && fs.statSync(base).isFile() ? base : null;
+}
+
+/**
+ * Walk the transitive import graph of a file and return all source-relative
+ * paths it depends on (files under src/ or open-sse/).
+ */
+export function sourceDepsOf(entry) {
+  const seen = new Set();
+  const stack = [entry];
+  const sources = new Set();
+  while (stack.length) {
+    const f = stack.pop();
+    if (seen.has(f)) continue;
+    seen.add(f);
+    let code;
+    try {
+      code = fs.readFileSync(f, "utf8");
+    } catch {
+      continue;
+    }
+    for (const m of code.matchAll(IMPORT_RE)) {
+      const spec = m[1] || m[2] || m[3];
+      if (!spec) continue;
+      const r = resolveImport(spec, f);
+      if (!r) continue;
+      const rel = path.relative(ROOT, r);
+      if (SRC_ROOTS.some((s) => rel.startsWith(s + path.sep))) sources.add(rel);
+      stack.push(r);
+    }
+  }
+  return sources;
+}

scripts/quality/build-test-impact-map.mjs

@@ -1,57 +1,14 @@
 import fs from "node:fs";
 import path from "node:path";
-import { fileURLToPath } from "node:url";
 import { globSync } from "tinyglobby";
-
-const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../..");
-const SRC_ROOTS = ["src", "open-sse"];
-const IMPORT_RE =
-  /(?:import|export)[^'"]*from\s*['"]([^'"]+)['"]|require\(\s*['"]([^'"]+)['"]\s*\)|import\(\s*['"]([^'"]+)['"]\s*\)/g;
-const EXTS = [".ts", ".tsx", ".mts", ".js", ".mjs"];
-
-function resolveImport(spec, fromFile) {
-  let base;
-  if (spec.startsWith("@/")) base = path.join(ROOT, "src", spec.slice(2));
-  else if (spec.startsWith("@omniroute/open-sse"))
-    base = path.join(ROOT, "open-sse", spec.replace(/^@omniroute\/open-sse\/?/, ""));
-  else if (spec.startsWith(".")) base = path.resolve(path.dirname(fromFile), spec);
-  else return null;
-  for (const e of EXTS) {
-    if (fs.existsSync(base + e)) return base + e;
-  }
-  for (const e of EXTS) {
-    const idx = path.join(base, "index" + e);
-    if (fs.existsSync(idx)) return idx;
-  }
-  return fs.existsSync(base) && fs.statSync(base).isFile() ? base : null;
-}
-
-function sourceDepsOf(entry) {
-  const seen = new Set();
-  const stack = [entry];
-  const sources = new Set();
-  while (stack.length) {
-    const f = stack.pop();
-    if (seen.has(f)) continue;
-    seen.add(f);
-    let code;
-    try {
-      code = fs.readFileSync(f, "utf8");
-    } catch {
-      continue;
-    }
-    for (const m of code.matchAll(IMPORT_RE)) {
-      const spec = m[1] || m[2] || m[3];
-      if (!spec) continue;
-      const r = resolveImport(spec, f);
-      if (!r) continue;
-      const rel = path.relative(ROOT, r);
-      if (SRC_ROOTS.some((s) => rel.startsWith(s + path.sep))) sources.add(rel);
-      stack.push(r);
-    }
-  }
-  return sources;
-}
+import {
+  ROOT,
+  IMPORT_RE,
+  EXTS,
+  SRC_ROOTS,
+  resolveImport,
+  sourceDepsOf,
+} from "../check/lib/importResolution.mjs";
 
 // Mirror EXACTLY the `npm run test:unit` glob — the curated set of node:test files.
 // The TIA step runs the selected subset via `node --test`, so it must NOT include
@@ -79,7 +36,10 @@ for (const tf of testFiles) {
 }
 for (const k of Object.keys(map)) map[k].sort();
 const out = path.join(ROOT, "config/quality/test-impact-map.json");
-fs.writeFileSync(out, JSON.stringify({ generatedFrom: "import-graph", sources: map }, null, 2) + "\n");
+fs.writeFileSync(
+  out,
+  JSON.stringify({ generatedFrom: "import-graph", sources: map }, null, 2) + "\n"
+);
 console.log(
   `test-impact-map: ${Object.keys(map).length} source files mapped from ${testFiles.length} test files`
 );

src/app/api/providers/[id]/test/route.ts

@@ -142,9 +142,6 @@ export function classifyFailure({
     normalized.includes("fetch failed") ||
     normalized.includes("network") ||
     normalized.includes("timeout") ||
-    // The OAuth probe reports its own abort as "Test timed out after 30s",
-    // which does not contain "timeout".
-    normalized.includes("timed out") ||
     normalized.includes("econn") ||
     normalized.includes("enotfound") ||
     normalized.includes("socket")
@@ -701,16 +698,8 @@ export async function testSingleConnection(connectionId: string, validationModel
       ? makeDiagnosis("ok", "local", null, null)
       : classifyFailure({ error: result.error, statusCode: result.statusCode, provider }));
 
-  // A network_error means the request never reached the upstream, so the test
-  // observed nothing about this connection and must not claim it is broken. The
-  // error fields below still record the attempt. Writing "error" here would be a
-  // one-way door: proactive recovery only restores connections that are
-  // "unavailable" AND carry an elapsed rateLimitedUntil, and a failed test sets
-  // neither, so a brief outage would leave the whole fleet red until re-tested
-  // by hand. See src/lib/quota/connectionRecovery.ts.
-  const observedTheConnection = diagnosis.code !== "network_error";
-
   const updateData: Record<string, any> = {
+    testStatus: result.valid ? "active" : "error",
     lastError: result.valid ? null : result.error,
     lastErrorAt: result.valid ? null : now,
     lastTested: now,
@@ -720,14 +709,6 @@ export async function testSingleConnection(connectionId: string, validationModel
     rateLimitedUntil: result.valid ? null : connection.rateLimitedUntil || null,
   };
 
-  // Only claim a status when the test actually observed the connection. On a
-  // network failure the key is left out entirely, and updateProviderConnection
-  // merges over the stored row, so the persisted status stays exactly as it was
-  // — including for a connection that has never been tested.
-  if (result.valid || observedTheConnection) {
-    updateData.testStatus = result.valid ? "active" : "error";
-  }
-
   if (result.valid) {
     updateData.backoffLevel = 0;
 

tests/unit/build/check-forgotten-sibling-tests.test.mjs

@@ -0,0 +1,130 @@
+// tests/unit/build/check-forgotten-sibling-tests.test.mjs
+// TDD tests for the forgotten-sibling-tests gate (check-forgotten-sibling-tests.mjs).
+
+import assert from "node:assert";
+import { describe, it } from "node:test";
+
+import {
+  testSiblingOf,
+  isAllowlisted,
+  isSourceFile,
+  resolveBase,
+  findConsumers,
+} from "../../../scripts/check/check-forgotten-sibling-tests.mjs";
+import { ROOT } from "../../../scripts/check/lib/importResolution.mjs";
+
+describe("check-forgotten-sibling-tests", () => {
+  describe("isSourceFile", () => {
+    it("returns true for src/ .ts files", () => {
+      assert.strictEqual(isSourceFile("src/lib/foo.ts"), true);
+    });
+    it("returns true for open-sse/ files", () => {
+      assert.strictEqual(isSourceFile("open-sse/services/foo.ts"), true);
+    });
+    it("returns true for bin/ files", () => {
+      assert.strictEqual(isSourceFile("bin/cli.ts"), true);
+    });
+    it("returns false for test files under tests/", () => {
+      assert.strictEqual(isSourceFile("tests/unit/foo.test.ts"), false);
+    });
+    it("returns false for migration files", () => {
+      assert.strictEqual(isSourceFile("src/lib/db/migrations/001.sql"), false);
+    });
+    it("returns false for node_modules", () => {
+      assert.strictEqual(isSourceFile("node_modules/foo/index.ts"), false);
+    });
+    it("returns false for markdown files", () => {
+      assert.strictEqual(isSourceFile("docs/readme.md"), false);
+    });
+  });
+
+  describe("resolveBase", () => {
+    it("returns GITHUB_BASE_SHA when set", () => {
+      const prev = process.env.GITHUB_BASE_SHA;
+      process.env.GITHUB_BASE_SHA = "abc123";
+      assert.strictEqual(resolveBase(), "abc123");
+      process.env.GITHUB_BASE_SHA = prev;
+    });
+    it("returns origin/REF when only GITHUB_BASE_REF is set", () => {
+      const prev = process.env.GITHUB_BASE_REF;
+      delete process.env.GITHUB_BASE_SHA;
+      process.env.GITHUB_BASE_REF = "release/v3.8.50";
+      assert.strictEqual(resolveBase(), "origin/release/v3.8.50");
+      process.env.GITHUB_BASE_REF = prev;
+    });
+    it("returns null when neither env var is set", () => {
+      const prevSha = process.env.GITHUB_BASE_SHA;
+      const prevRef = process.env.GITHUB_BASE_REF;
+      delete process.env.GITHUB_BASE_SHA;
+      delete process.env.GITHUB_BASE_REF;
+      assert.strictEqual(resolveBase(), null);
+      process.env.GITHUB_BASE_SHA = prevSha;
+      process.env.GITHUB_BASE_REF = prevRef;
+    });
+  });
+
+  describe("testSiblingOf", () => {
+    it("returns null for a file that has no test sibling", () => {
+      const result = testSiblingOf("src/lib/db/core.ts");
+      assert.strictEqual(result, null);
+    });
+  });
+
+  describe("isAllowlisted", () => {
+    const allowlist = [
+      {
+        sourcePath: "src/lib/example.ts",
+        forgottenSibling: "tests/unit/lib/consumer.test.ts",
+        reason: "test",
+      },
+      {
+        sourcePath: "*",
+        forgottenSibling: "tests/unit/lib/wildcard.test.ts",
+        reason: "wildcard test",
+      },
+    ];
+    it("returns true for exact match", () => {
+      assert.strictEqual(
+        isAllowlisted("src/lib/example.ts", "tests/unit/lib/consumer.test.ts", allowlist),
+        true
+      );
+    });
+    it("returns true for wildcard sourcePath", () => {
+      assert.strictEqual(
+        isAllowlisted("src/lib/other.ts", "tests/unit/lib/wildcard.test.ts", allowlist),
+        true
+      );
+    });
+    it("returns false for non-allowlisted pair", () => {
+      assert.strictEqual(
+        isAllowlisted("src/lib/example.ts", "tests/unit/lib/other.test.ts", allowlist),
+        false
+      );
+    });
+    it("returns false for empty allowlist", () => {
+      assert.strictEqual(
+        isAllowlisted("src/lib/example.ts", "tests/unit/lib/consumer.test.ts", []),
+        false
+      );
+    });
+  });
+
+  describe("findConsumers", () => {
+    it("returns consumers that import the given file", () => {
+      const absPath = ROOT + "/src/lib/target.ts";
+      const prodFileMap = {
+        "src/lib/consumer.ts": new Set([absPath]),
+        "src/lib/unrelated.ts": new Set(["/other/path.ts"]),
+      };
+      const result = findConsumers("src/lib/target.ts", prodFileMap);
+      assert.deepStrictEqual(result, ["src/lib/consumer.ts"]);
+    });
+    it("returns empty array when no consumers import the file", () => {
+      const prodFileMap = {
+        "src/lib/consumer.ts": new Set([ROOT + "/src/lib/other.ts"]),
+      };
+      const result = findConsumers("src/lib/target.ts", prodFileMap);
+      assert.deepStrictEqual(result, []);
+    });
+  });
+});

tests/unit/connection-test-network-error-status-9623.test.ts

@@ -1,198 +0,0 @@
-/**
- * Regression for #9623: a connection test that fails because the request never
- * left the host must not persist testStatus='error'.
- *
- * Bug: testSingleConnection() wrote `testStatus: result.valid ? "active" : "error"`
- * for every failure, including the `network_error` diagnosis that classifyFailure()
- * returns for "fetch failed" / ENOTFOUND / ECONNREFUSED / timeouts. Those failures
- * mean the request never reached the upstream, so the test observed nothing about
- * the connection itself.
- *
- * That mattered because 'error' has no way back. Proactive recovery
- * (src/lib/quota/connectionRecovery.ts) restores a connection only when BOTH gates
- * pass: testStatus === 'unavailable' (line 85) AND an elapsed rateLimitedUntil
- * (line 87 — hasElapsedCooldown returns false on null). A failed test sets neither:
- * it writes 'error' and carries the previous rateLimitedUntil forward, which is null
- * for a healthy connection. So the rows fail both gates and stay red until someone
- * re-tests by hand. Measured after a host reboot: 20 connections across 6 providers
- * went red inside 0.823s and were still red 63 minutes later.
- *
- * Fix: keep whatever status the connection already had when the diagnosis is
- * network_error. The error fields still record the attempt, matching how
- * src/lib/tokenHealthCheck.ts already handles a transient refresh failure.
- *
- * This drives the REAL (unmocked) testSingleConnection() against a temp SQLite DB,
- * following tests/unit/apikey-connection-health-check.test.ts and
- * tests/unit/token-health-check-sweep.test.ts, since mock.module() is unavailable
- * in this tsx/ESM + Node native test-runner setup. Driving the whole function
- * rather than an extracted helper is deliberate: it is what makes this fail if the
- * write path stops consulting the diagnosis.
- *
- * The connection is OAuth/github because that path reaches a bare fetch() that a
- * stub can drive (same approach as tests/unit/oauth-connection-test-timeout.test.ts).
- * API-key providers return "Provider test not supported" here, since the provider
- * registry is not populated under the unit-test runner.
- */
-import test from "node:test";
-import assert from "node:assert/strict";
-import fs from "node:fs";
-import os from "node:os";
-import path from "node:path";
-
-process.env.NODE_ENV = "test";
-
-const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-9623-"));
-process.env.DATA_DIR = TEST_DATA_DIR;
-
-const core = await import("../../src/lib/db/core.ts");
-const providersDb = await import("../../src/lib/db/providers.ts");
-const { testSingleConnection } = await import("../../src/app/api/providers/[id]/test/route.ts");
-
-async function resetStorage() {
-  core.resetDbInstance();
-  for (let attempt = 0; attempt < 10; attempt++) {
-    try {
-      if (fs.existsSync(TEST_DATA_DIR)) {
-        fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
-      }
-      break;
-    } catch (error) {
-      const code = (error as { code?: string } | undefined)?.code;
-      if ((code === "EBUSY" || code === "EPERM") && attempt < 9) {
-        await new Promise((resolve) => setTimeout(resolve, 50 * (attempt + 1)));
-      } else {
-        throw error;
-      }
-    }
-  }
-  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
-}
-
-test.after(() => {
-  core.resetDbInstance();
-  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
-});
-
-/** An OAuth connection whose probe goes through a bare fetch() a stub can drive. */
-async function createHealthyConnection(name: string) {
-  return providersDb.createProviderConnection({
-    provider: "github",
-    name,
-    authType: "oauth",
-    accessToken: "fake-token-for-test",
-    refreshToken: null,
-    expiresAt: new Date(Date.now() + 60 * 60 * 1000).toISOString(),
-    isActive: true,
-    testStatus: "active",
-  });
-}
-
-/** Replace fetch for one test and restore it afterwards. */
-function stubFetch(t: { after: (fn: () => void) => void }, impl: () => Promise<Response>) {
-  const original = globalThis.fetch;
-  t.after(() => {
-    globalThis.fetch = original;
-  });
-  globalThis.fetch = impl as unknown as typeof fetch;
-}
-
-test("#9623: a network failure leaves testStatus alone instead of writing 'error'", async (t) => {
-  await resetStorage();
-
-  const conn = await createHealthyConnection("github-network-error-9623");
-  assert.equal(conn.testStatus, "active", "precondition: connection starts active");
-
-  // The shape undici produces when the host cannot reach the network at all.
-  stubFetch(t, () => Promise.reject(new TypeError("fetch failed")));
-
-  const result = await testSingleConnection(conn.id);
-  assert.equal(result.valid, false, "precondition: the test must have failed");
-  assert.equal(
-    result.diagnosis?.code,
-    "network_error",
-    "precondition: the failure must be diagnosed as a network error"
-  );
-
-  const updated = await providersDb.getProviderConnectionById(conn.id);
-
-  assert.equal(
-    updated?.testStatus,
-    "active",
-    "a failure that never reached the upstream must not overwrite the connection status"
-  );
-  assert.equal(
-    updated?.errorCode,
-    "network_error",
-    "the failed attempt is still recorded, so the operator can see the test did not succeed"
-  );
-  assert.ok(updated?.lastError, "lastError still carries the underlying message");
-  assert.equal(
-    updated?.rateLimitedUntil ?? null,
-    null,
-    "no cooldown is invented for a failure the connection did not cause"
-  );
-});
-
-test("#9623: a probe that times out is also treated as never reaching the upstream", async (t) => {
-  await resetStorage();
-
-  const conn = await createHealthyConnection("github-timeout-9623");
-
-  // testOAuthConnection turns an AbortSignal.timeout() abort into its own message,
-  // "Test timed out after 30s" — which does not contain the substring "timeout",
-  // so classifyFailure used to fall through to a generic upstream_error and the
-  // connection was marked broken by a hang it never caused.
-  stubFetch(t, () => {
-    const err = new Error("The operation was aborted due to timeout");
-    err.name = "TimeoutError";
-    return Promise.reject(err);
-  });
-
-  const result = await testSingleConnection(conn.id);
-  assert.equal(result.valid, false, "precondition: the test must have failed");
-  assert.match(
-    String(result.error),
-    /timed out/i,
-    "precondition: the OAuth probe reports its abort in its own wording"
-  );
-  assert.equal(
-    result.diagnosis?.code,
-    "network_error",
-    "a timed-out probe never reached the upstream, so it is a network failure"
-  );
-
-  const updated = await providersDb.getProviderConnectionById(conn.id);
-  assert.equal(updated?.testStatus, "active", "a hang must not mark the connection broken");
-});
-
-test("#9623: a real upstream rejection still marks the connection as error", async (t) => {
-  await resetStorage();
-
-  const conn = await createHealthyConnection("github-auth-error-9623");
-
-  // A 401 is the upstream answering, so the test DID observe the connection.
-  stubFetch(t, () =>
-    Promise.resolve(
-      new Response(JSON.stringify({ message: "Bad credentials" }), {
-        status: 401,
-        headers: { "content-type": "application/json" },
-      })
-    )
-  );
-
-  const result = await testSingleConnection(conn.id);
-  assert.equal(result.valid, false, "precondition: the test must have failed");
-  assert.notEqual(
-    result.diagnosis?.code,
-    "network_error",
-    "precondition: an answered 401 is not a network failure"
-  );
-
-  const updated = await providersDb.getProviderConnectionById(conn.id);
-
-  assert.equal(
-    updated?.testStatus,
-    "error",
-    "an answered rejection is a real observation and must still mark the connection"
-  );
-});

tests/unit/repro-9630-combo-false-503.test.ts

@@ -1,10 +1,17 @@
 import test from "node:test";
 import assert from "node:assert/strict";
+import type {
+  ComboLogger,
+  ComboRelayOptions,
+} from "../../open-sse/services/combo/types.ts";
 import {
   handleComboChat,
 } from "../../open-sse/services/combo.ts";
 import { getCircuitBreaker, STATE } from "../../src/shared/utils/circuitBreaker.js";
 
+const noopLogger: ComboLogger = { info: () => {}, warn: () => {}, debug: () => {}, error: () => {} };
+type BodyType = Record<string, unknown>;
+
 function okResponse() {
   return new Response(JSON.stringify({ ok: true }), {
     status: 200,
@@ -27,14 +34,14 @@ test("#9630: combo returns 503 when circuit breaker is OPEN but other healthy ta
       strategy: "priority",
       models: ["openai/gpt-4", "anthropic/claude-opus-5"],
     },
-    handleSingleModel: async (_body: any, modelStr: string) => {
+    handleSingleModel: async (_body: BodyType, modelStr: string) => {
       assert.equal(modelStr, "anthropic/claude-opus-5", "should skip openai breaker and try anthropic");
       return okResponse();
     },
     isModelAvailable: async () => true,
-    log: { info: () => {}, warn: () => {}, debug: () => {}, error: () => {} } as any,
+    log: noopLogger,
     settings: null,
-    relayOptions: null as any,
+    relayOptions: null as unknown as ComboRelayOptions,
     allCombos: null,
   });
 
@@ -65,9 +72,9 @@ test("#9630: combo returns truthful error, not false ALL_ACCOUNTS_INACTIVE, when
     },
     handleSingleModel: async () => { throw new Error("should not be called"); },
     isModelAvailable: async () => true,
-    log: { info: () => {}, warn: () => {}, debug: () => {}, error: () => {} } as any,
+    log: noopLogger,
     settings: null,
-    relayOptions: null as any,
+    relayOptions: null as unknown as ComboRelayOptions,
     allCombos: null,
   });