From ee3fbaf3f6ab964f2163e255f4ca9f4e6bcf6de0 Mon Sep 17 00:00:00 2001 From: Dizzle <112548150+maxmad64bis@users.noreply.github.com> Date: Tue, 15 Sep 2026 15:11:44 +0200 Subject: [PATCH] fix(proxies): preserve inactive and dead statuses during pool validation (#13612) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pool validation no longer rewrites `inactive` or `dead` proxies to `active`: `validateProxyPool` only ever touched rows that were already live. Covered by 8 status × probe combinations plus case and null variants. Validated first on the combined board of all 38 PRs of this batch (10 merged as-is, 28 after the maintainer rework) on top of release/v3.8.51 c0f92ec: typecheck:core, check:open-sse-typecheck and check:dashboard-typecheck clean; ESLint clean on every changed file; file-size (rebaselined for the combined growth), complexity, cognitive-complexity, changelog-integrity, docs-counts, docs-sync, migration-numbering and i18n new-key gates green; 735 focused node:test cases with the only batch-caused failure (a flag-count assertion) fixed. Then re-validated alone on the fresh release tip right before this merge: ESLint on the changed files, typecheck:core, check:open-sse-typecheck, the file-size/complexity/changelog gates and this PR's own tests. Thanks @maxmad64bis! --- .../13612-validate-pool-preserve-status.md | 1 + src/lib/proxyEgress.ts | 23 +- ...roxy-egress-validate-pool-preserve.test.ts | 198 ++++++++++++++++++ 3 files changed, 221 insertions(+), 1 deletion(-) create mode 100644 changelog.d/fixes/13612-validate-pool-preserve-status.md create mode 100644 tests/unit/proxy-egress-validate-pool-preserve.test.ts diff --git a/changelog.d/fixes/13612-validate-pool-preserve-status.md b/changelog.d/fixes/13612-validate-pool-preserve-status.md new file mode 100644 index 0000000000..3198561fc0 --- /dev/null +++ b/changelog.d/fixes/13612-validate-pool-preserve-status.md @@ -0,0 +1 @@ +- **fix(proxies):** pool validation no longer rewrites proxies set to inactive or dead; only active and error statuses are updated ([#13612](https://github.com/diegosouzapw/OmniRoute/pull/13612)) — thanks @maxmad64bis diff --git a/src/lib/proxyEgress.ts b/src/lib/proxyEgress.ts index af7ab5ded5..5a11ace55e 100644 --- a/src/lib/proxyEgress.ts +++ b/src/lib/proxyEgress.ts @@ -363,7 +363,8 @@ export interface ProxyValidationResult { egressIp: string | null; latencyMs: number; previousStatus: string | null; - newStatus: "active" | "error"; + newStatus: string; + preserved: boolean; } /** @@ -428,6 +429,25 @@ export async function validateProxyPool(deps?: { }); const probe = await resolveEgressIp(url, { force: true }); const alive = !!probe.ip && !probe.error; + // Operator/health statuses stay untouched by validation: the probe still + // ran above, so the report keeps its alive/egressIp signal. The two-value + // literal mirrors PROXY_ALIVE_PREDICATE in db/proxies/guards.ts; revisit if + // the status registry is ever derived from a single shared source (part 2). + const previous = (p.status ?? "").toLowerCase(); + if (previous === "inactive" || previous === "dead") { + report.push({ + proxyId: p.id, + host: p.host, + port: p.port, + alive, + egressIp: probe.ip, + latencyMs: probe.latencyMs, + previousStatus: p.status ?? null, + newStatus: p.status as string, + preserved: true, + }); + continue; + } const newStatus: "active" | "error" = alive ? "active" : "error"; await markStatus(p.id, newStatus, { latencyMs: probe.latencyMs, egressIp: probe.ip }); report.push({ @@ -439,6 +459,7 @@ export async function validateProxyPool(deps?: { latencyMs: probe.latencyMs, previousStatus: p.status ?? null, newStatus, + preserved: false, }); } diff --git a/tests/unit/proxy-egress-validate-pool-preserve.test.ts b/tests/unit/proxy-egress-validate-pool-preserve.test.ts new file mode 100644 index 0000000000..e6d3866e78 --- /dev/null +++ b/tests/unit/proxy-egress-validate-pool-preserve.test.ts @@ -0,0 +1,198 @@ +/** + * Pool validation must preserve operator/health statuses. + * + * Validating the pool rewrites only the transient active <-> error pair. A + * proxy set aside as `inactive` (operator) or marked `dead` (health) keeps its + * status even when the probe result disagrees — the probe still runs so the + * report keeps the alive/egressIp signal. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const egress = await import("../../src/lib/proxyEgress.ts"); +const { validateProxyPool, _setEgressProbeForTests, clearEgressCache } = egress; + +type Deps = NonNullable[0]>; +type ProxyRow = { id: string; type: string; host: string; port: number; status?: string | null }; +type MarkCall = { id: string; status: string }; + +const LIVE_IP = "198.51.100.21"; + +function liveOrDeadProbe( + proxyUrl: string | null +): Promise<{ ip: string | null; latencyMs: number; error?: string }> { + if (proxyUrl && proxyUrl.includes("-up.")) return Promise.resolve({ ip: LIVE_IP, latencyMs: 4 }); + return Promise.resolve({ ip: null, latencyMs: 7000, error: "timeout" }); +} + +function proxyRow(id: string, status: string | null, live: boolean): ProxyRow { + return { id, type: "http", host: `${id}-${live ? "up" : "down"}.local`, port: 8080, status }; +} + +test.afterEach(() => { + _setEgressProbeForTests(null); + clearEgressCache(); +}); + +test("validateProxyPool rewrites active/error but preserves inactive/dead", async () => { + clearEgressCache(); + _setEgressProbeForTests(liveOrDeadProbe); + + const cases: Array<{ + previous: string | null; + live: boolean; + expectWrite: string | null; + expectNewStatus: string; + expectPreserved: boolean; + }> = [ + { + previous: "active", + live: true, + expectWrite: "active", + expectNewStatus: "active", + expectPreserved: false, + }, + { + previous: "active", + live: false, + expectWrite: "error", + expectNewStatus: "error", + expectPreserved: false, + }, + { + previous: "error", + live: true, + expectWrite: "active", + expectNewStatus: "active", + expectPreserved: false, + }, + { + previous: "error", + live: false, + expectWrite: "error", + expectNewStatus: "error", + expectPreserved: false, + }, + { + previous: "inactive", + live: true, + expectWrite: null, + expectNewStatus: "inactive", + expectPreserved: true, + }, + { + previous: "inactive", + live: false, + expectWrite: null, + expectNewStatus: "inactive", + expectPreserved: true, + }, + { + previous: "dead", + live: true, + expectWrite: null, + expectNewStatus: "dead", + expectPreserved: true, + }, + { + previous: "dead", + live: false, + expectWrite: null, + expectNewStatus: "dead", + expectPreserved: true, + }, + // Mixed casing still matches, original casing echoed back. + { + previous: "Inactive", + live: true, + expectWrite: null, + expectNewStatus: "Inactive", + expectPreserved: true, + }, + // Unknown status routes to the rewritable branch and gets written. + { + previous: null, + live: true, + expectWrite: "active", + expectNewStatus: "active", + expectPreserved: false, + }, + ]; + + const listProxies: Deps["listProxies"] = async () => + cases.map((c, i) => proxyRow(`case-${i}`, c.previous, c.live)); + const calls: MarkCall[] = []; + const markStatus: Deps["markStatus"] = async (id, status) => { + calls.push({ id, status }); + }; + + const report = await validateProxyPool({ listProxies, markStatus }); + + assert.equal(report.length, cases.length); + for (let i = 0; i < cases.length; i++) { + const row = report[i]; + const c = cases[i]; + assert.equal(row.proxyId, `case-${i}`); + assert.equal(row.previousStatus, c.previous); + assert.equal(row.alive, c.live); + assert.equal(row.newStatus, c.expectNewStatus); + assert.equal(row.preserved, c.expectPreserved); + assert.equal( + row.egressIp, + c.live ? LIVE_IP : null, + "probe signal is reported even for preserved rows" + ); + } + + const expectedWrites = cases.filter((c) => c.expectWrite !== null).length; + assert.equal(calls.length, expectedWrites, "markStatus runs only for rewritable rows"); + for (let i = 0; i < cases.length; i++) { + const expected = cases[i].expectWrite; + if (expected === null) { + assert.ok( + !calls.some((c) => c.id === `case-${i}`), + `preserved row case-${i} must not be rewritten` + ); + } else { + assert.equal(calls.find((c) => c.id === `case-${i}`)?.status, expected); + } + } +}); + +test("validateProxyPool on a mixed pool leaves operator/health statuses intact", async () => { + clearEgressCache(); + _setEgressProbeForTests(liveOrDeadProbe); + + const listProxies: Deps["listProxies"] = async () => [ + proxyRow("pool-active", "active", true), + proxyRow("pool-error", "error", false), + proxyRow("pool-inactive", "inactive", true), + proxyRow("pool-dead", "dead", true), + ]; + const calls: MarkCall[] = []; + const markStatus: Deps["markStatus"] = async (id, status) => { + calls.push({ id, status }); + }; + + const report = await validateProxyPool({ listProxies, markStatus }); + + const byId = new Map(report.map((r) => [r.proxyId, r])); + assert.equal(byId.get("pool-active")?.newStatus, "active"); + assert.equal(byId.get("pool-active")?.preserved, false); + assert.equal(byId.get("pool-error")?.newStatus, "error"); + assert.equal(byId.get("pool-error")?.preserved, false); + assert.equal(byId.get("pool-inactive")?.newStatus, "inactive"); + assert.equal(byId.get("pool-inactive")?.preserved, true); + assert.equal(byId.get("pool-inactive")?.alive, true); + assert.equal(byId.get("pool-inactive")?.egressIp, LIVE_IP); + assert.equal(byId.get("pool-dead")?.newStatus, "dead"); + assert.equal(byId.get("pool-dead")?.preserved, true); + assert.equal(byId.get("pool-dead")?.alive, true); + assert.equal(byId.get("pool-dead")?.egressIp, LIVE_IP); + + assert.deepEqual( + calls.map((c) => c.id).sort(), + ["pool-active", "pool-error"], + "only rewritable rows are persisted" + ); +});