From 47155fa3a0f2533f29bb86eb35ddcc2cc6b45d82 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Thu, 11 Jun 2026 12:55:02 -0300 Subject: [PATCH] fix(api): flag provider topology error state by current status, not stale history (#3619) (#3666) Closes #3619 --- CHANGELOG.md | 1 + src/app/api/provider-metrics/route.ts | 7 ++++- tests/unit/provider-metrics-route.test.ts | 37 +++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06e96885fe..0d0e22a679 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ ### 🔧 Bug Fixes +- **Provider Topology**: stop flagging healthy providers as errored based on stale historical failures; use current request status (#3619) - **OpenCode Free**: fetch the live model catalog from the provider's `modelsUrl` for the no-auth model picker instead of serving a stale hardcoded list (#3611) - **Hermes Agent**: honour the `HERMES_HOME` env var when writing/reading the agent config instead of always using `~/.hermes` (#3628). Introduced `getHermesHome()` / `getHermesConfigPath()` helpers (read at call-time) and routed all four hardcoded callsites through them so OmniRoute's config lands in the same directory that the Hermes PowerShell installer configures on Windows. - **MITM/cert**: remove the duplicated "Command failed:" prefix in system-command error messages ([#3641](https://github.com/diegosouzapw/OmniRoute/issues/3641)): `execFileText` was prepending its own `"Command failed: "` prefix on top of Node's `execFile` error message, which already begins with `"Command failed: "` for non-zero exits. The error message now surfaces Node's message directly (no double prefix), with stderr appended only when non-empty. diff --git a/src/app/api/provider-metrics/route.ts b/src/app/api/provider-metrics/route.ts index 38a16da010..d849cb9a9e 100644 --- a/src/app/api/provider-metrics/route.ts +++ b/src/app/api/provider-metrics/route.ts @@ -71,7 +71,12 @@ export async function GET() { lastProviderTs = requestTs; } - const errorTs = lastErrorAt ? Date.parse(lastErrorAt) : 0; + // Only flag as errorProvider if the provider's MOST RECENT request was itself + // a failure. A provider with a historical lastErrorAt but a recent success + // (lastStatus 2xx/3xx) must not be shown as currently errored (#3619). + const isCurrentlyInError = + lastStatus !== null && (lastStatus < 200 || lastStatus >= 400); + const errorTs = isCurrentlyInError && lastErrorAt ? Date.parse(lastErrorAt) : 0; if (Number.isFinite(errorTs) && errorTs > errorProviderTs) { errorProvider = provider; errorProviderTs = errorTs; diff --git a/tests/unit/provider-metrics-route.test.ts b/tests/unit/provider-metrics-route.test.ts index e56a71afea..ca5def5bb5 100644 --- a/tests/unit/provider-metrics-route.test.ts +++ b/tests/unit/provider-metrics-route.test.ts @@ -85,6 +85,43 @@ test("GET /api/provider-metrics includes provider recency and error topology", a assert.equal(body.topology.errorProvider, "openai"); }); +test("GET /api/provider-metrics errorProvider must NOT flag a provider whose most recent request succeeded", async () => { + // Arrange: providerA had an error long ago but recovered (last request = 200). + // providerB never errored. + // Bug (pre-fix): errorProvider = "providerA" because lastErrorAt > 0. + // Fix (post-fix): errorProvider = "" because lastStatus for providerA is 200. + const db = core.getDbInstance(); + db.prepare( + `INSERT INTO call_logs (id, timestamp, provider, status, duration, error_summary) + VALUES (?, ?, ?, ?, ?, ?)` + ).run("a-old-error", "2026-01-01T00:00:00.000Z", "providerA", 500, 100, "old error"); + db.prepare( + `INSERT INTO call_logs (id, timestamp, provider, status, duration, error_summary) + VALUES (?, ?, ?, ?, ?, ?)` + ).run("a-recent-ok", "2026-06-01T00:00:00.000Z", "providerA", 200, 80, null); + db.prepare( + `INSERT INTO call_logs (id, timestamp, provider, status, duration, error_summary) + VALUES (?, ?, ?, ?, ?, ?)` + ).run("b-ok", "2026-05-01T00:00:00.000Z", "providerB", 200, 60, null); + + const response = await providerMetricsRoute.GET(); + const body = (await response.json()) as ProviderMetricsResponse; + + assert.equal(response.status, 200); + // providerA's last request succeeded — must NOT appear as errorProvider + assert.notEqual( + body.topology.errorProvider, + "providerA", + "providerA recovered (lastStatus=200) and must not be marked as errorProvider" + ); + // No provider is currently in error — errorProvider should be empty + assert.equal( + body.topology.errorProvider, + "", + "errorProvider must be empty when no provider's most recent request was a failure" + ); +}); + test("GET /api/provider-metrics returns sanitized 500 when metrics cannot be loaded", async () => { const db = core.getDbInstance(); db.close();