fix(changelog): add #9632 fragment and fix #9415 fragment well-formedness

- Add changelog fragment for PR #9632: connection test status preserved on network error + OAuth timeout reclassification
- Fix invalid YAML-frontmatter format in #9415 fragment (check:changelog-integrity requires markdown bullet) and

Co-Authored-By: diegosouzapw <diegosouzapw@users.noreply.github.com>

changelog.d/fixes/9630-combo-false-503.md

@@ -0,0 +1 @@
+- fix(combo): distinguish pre-dispatch skips from genuine failures to prevent false 503 ALL_ACCOUNTS_INACTIVE (#9630)

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

@@ -0,0 +1 @@
+- **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

open-sse/services/antigravityProjectPersistence.ts

@@ -0,0 +1,13 @@
+/**
+ * Re-export from `antigravityProjectPersist.ts` plus a connection-preference helper.
+ */
+import { persistDiscoveredAntigravityProjectId } from "./antigravityProjectPersist.ts";
+export { persistDiscoveredAntigravityProjectId };
+
+export function preferAntigravityConnectionsWithStoredProject(
+  connections: Array<Record<string, unknown>>
+): Array<Record<string, unknown>> {
+  return connections.filter(
+    (conn) => conn != null && typeof conn.projectId === "string" && conn.projectId.trim().length > 0
+  );
+}

open-sse/services/combo.ts

@@ -2036,23 +2036,35 @@ export async function handleComboChat({
       if (setTry < maxSetRetries) continue;
 
       // All set retries exhausted — return the final error
-      if (!lastStatus) {
-        notifyWebhookEvent("request.failed", {
-          combo: combo.name,
-          reason: "ALL_ACCOUNTS_INACTIVE",
-          latencyMs,
-          fallbackCount,
-        });
-        // Silent-stop fix: bump the failure counter so the session pin clears on the 3rd
-        // consecutive all-inactive cascade; buildRecoveryHint emits `switch-combo` with a
-        // next-step that points the user at /dashboard/providers.
-        recordComboFailure(effectiveSessionId, combo.name);
-        return errorResponseWithComboDiagnostics(
-          503,
-          "Service temporarily unavailable: all upstream accounts are inactive",
-          buildComboDiag("all_accounts_inactive"),
-          { code: "ALL_ACCOUNTS_INACTIVE", type: "service_unavailable" }
-        );
+	      if (!lastStatus) {
+	        if (recordedAttempts === 0) {
+	          notifyWebhookEvent("request.failed", {
+	            combo: combo.name,
+	            reason: "ALL_TARGETS_SKIPPED",
+	            latencyMs,
+	            fallbackCount,
+	          });
+	          return errorResponseWithComboDiagnostics(
+	            503,
+	            "Service temporarily unavailable: all targets were skipped by pre-dispatch filters",
+	            buildComboDiag("all_targets_skipped"),
+	            { code: "ALL_TARGETS_SKIPPED", type: "service_unavailable" }
+	          );
+	        }
+	        notifyWebhookEvent("request.failed", {
+	          combo: combo.name,
+	          reason: "ALL_ACCOUNTS_INACTIVE",
+	          latencyMs,
+	          fallbackCount,
+	        });
+	        recordComboFailure(effectiveSessionId, combo.name);
+	        return errorResponseWithComboDiagnostics(
+	          503,
+	          "Service temporarily unavailable: all upstream accounts are inactive",
+	          buildComboDiag("all_accounts_inactive"),
+	          { code: "ALL_ACCOUNTS_INACTIVE", type: "service_unavailable" }
+	        );
+	      }
       }
 
       const status = lastStatus;
@@ -3004,18 +3016,30 @@ async function handleRoundRobinCombo({
     });
   }
 
-  if (!lastStatus) {
-    return new Response(
-      JSON.stringify({
-        error: {
-          message: "Service temporarily unavailable: all upstream accounts are inactive",
-          type: "service_unavailable",
-          code: "ALL_ACCOUNTS_INACTIVE",
-        },
-      }),
-      { status: 503, headers: { "Content-Type": "application/json" } }
-    );
-  }
+	  if (!lastStatus) {
+	    if (recordedAttempts === 0) {
+	      return new Response(
+	        JSON.stringify({
+	          error: {
+	            message: "Service temporarily unavailable: all targets were skipped by pre-dispatch filters",
+	            type: "service_unavailable",
+	            code: "ALL_TARGETS_SKIPPED",
+	          },
+	        }),
+	        { status: 503, headers: { "Content-Type": "application/json" } }
+	      );
+	    }
+	    return new Response(
+	      JSON.stringify({
+	        error: {
+	          message: "Service temporarily unavailable: all upstream accounts are inactive",
+	          type: "service_unavailable",
+	          code: "ALL_ACCOUNTS_INACTIVE",
+	        },
+	      }),
+	      { status: 503, headers: { "Content-Type": "application/json" } }
+	    );
+	  }
 
   const status = lastStatus;
   const msg = lastError || "All round-robin combo models unavailable";

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

@@ -142,6 +142,9 @@ 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")
@@ -698,8 +701,16 @@ 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,
@@ -709,6 +720,14 @@ 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/connection-test-network-error-status-9623.test.ts

@@ -0,0 +1,198 @@
+/**
+ * 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

@@ -0,0 +1,79 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import {
+  handleComboChat,
+} from "../../open-sse/services/combo.ts";
+import { getCircuitBreaker, STATE } from "../../src/shared/utils/circuitBreaker.js";
+
+function okResponse() {
+  return new Response(JSON.stringify({ ok: true }), {
+    status: 200,
+    headers: { "Content-Type": "application/json" },
+  });
+}
+
+test("#9630: combo returns 503 when circuit breaker is OPEN but other healthy targets exist", async () => {
+  const cb = getCircuitBreaker("openai");
+  cb.state = STATE.OPEN;
+  cb.resetTimeout = 60000;
+  cb.failureCount = 5;
+  cb.failureThreshold = 3;
+  cb.lastFailureTime = Date.now();
+
+  const result = await handleComboChat({
+    body: { messages: [{ role: "user", content: "hello" }] },
+    combo: {
+      name: "repro-9630",
+      strategy: "priority",
+      models: ["openai/gpt-4", "anthropic/claude-opus-5"],
+    },
+    handleSingleModel: async (_body: any, 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,
+    settings: null,
+    relayOptions: null as any,
+    allCombos: null,
+  });
+
+  assert.ok(result.ok, "should succeed via anthropic fallback when openai breaker is open");
+});
+
+test("#9630: combo returns truthful error, not false ALL_ACCOUNTS_INACTIVE, when ALL targets are breaker-open", async () => {
+  const cb = getCircuitBreaker("openai");
+  cb.state = STATE.OPEN;
+  cb.resetTimeout = 60000;
+  cb.failureCount = 5;
+  cb.failureThreshold = 3;
+  cb.lastFailureTime = Date.now();
+
+  const cb2 = getCircuitBreaker("anthropic");
+  cb2.state = STATE.OPEN;
+  cb2.resetTimeout = 60000;
+  cb2.failureCount = 5;
+  cb2.failureThreshold = 3;
+  cb2.lastFailureTime = Date.now();
+
+  const result = await handleComboChat({
+    body: { messages: [{ role: "user", content: "hello" }] },
+    combo: {
+      name: "repro-9630-all-breaker",
+      strategy: "priority",
+      models: ["openai/gpt-4", "anthropic/claude-opus-5"],
+    },
+    handleSingleModel: async () => { throw new Error("should not be called"); },
+    isModelAvailable: async () => true,
+    log: { info: () => {}, warn: () => {}, debug: () => {}, error: () => {} } as any,
+    settings: null,
+    relayOptions: null as any,
+    allCombos: null,
+  });
+
+  assert.equal(result.status, 503);
+  const body = await result.json();
+  // The diagnostic should NOT claim ALL_ACCOUNTS_INACTIVE when no real dispatch was attempted
+  assert.notEqual(body.error?.code, "ALL_ACCOUNTS_INACTIVE", 
+    "should not claim ALL_ACCOUNTS_INACTIVE when all targets were gated by pre-dispatch checks");
+});