Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: <cmd>"` for non-zero exits. The error message now surfaces Node's message directly (no double prefix), with stderr appended only when non-empty.
Expand Down
7 changes: 6 additions & 1 deletion src/app/api/provider-metrics/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Comment on lines +77 to +78

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To ensure robust handling of both null and undefined values, it is safer and more idiomatic in TypeScript/JavaScript to use loose inequality (!= null) instead of strict inequality (!== null). This prevents potential issues if lastStatus is ever missing or undefined.

Suggested change
const isCurrentlyInError =
lastStatus !== null && (lastStatus < 200 || lastStatus >= 400);
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;
Expand Down
37 changes: 37 additions & 0 deletions tests/unit/provider-metrics-route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down