Repository navigation
feat(proxy): batch delete, auto-test, health scheduler + fix alias resolution - #5918
Conversation
There was a problem hiding this comment.
Code Review
This pull request integrates the 1ai-ecosystem engineering rules, introduces a background proxy health check scheduler to auto-remove dead proxies, and adds batch delete and auto-test capabilities to the proxy registry. Feedback on these changes identifies several critical issues: a runtime ReferenceError in the auto-test route due to an undeclared alive variable, potential resource leaks in both the auto-test route and the scheduler due to uncleared timeout timers on fetch errors, a portability issue in docker-compose.prod.yml from a hardcoded host path, missing cache invalidation when proxies are auto-removed by the scheduler, and hardcoded English UI strings in the Proxy Registry Manager that violate i18n standards.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| async function testSingleProxy(proxy: { id: string; type: string; host: string; port: number }): Promise<TestResult> { | ||
| const proxyUrl = `${proxy.type}://${proxy.host}:${proxy.port}`; | ||
| const start = Date.now(); | ||
|
|
||
| try { | ||
| const controller = new AbortController(); | ||
| const timeout = setTimeout(() => controller.abort(), TEST_TIMEOUT_MS); | ||
| const dispatcher = createProxyDispatcher(proxyUrl); | ||
| const resp = await undiciFetch(TEST_URL, { | ||
| method: "HEAD", | ||
| signal: controller.signal, | ||
| dispatcher, | ||
| headers: { "User-Agent": "OmniRoute/1.0" }, | ||
| }); | ||
| clearTimeout(timeout); | ||
| const latencyMs = Date.now() - start; | ||
| await updateProxy(proxy.id, { status: alive ? "active" : "inactive" }).catch(() => {}); | ||
| return { proxyId: proxy.id, host: proxy.host, port: proxy.port, alive, latencyMs }; | ||
| } catch (err) { | ||
| const latencyMs = Date.now() - start; | ||
| await updateProxy(proxy.id, { status: "inactive" }).catch(() => {}); | ||
| return { | ||
| proxyId: proxy.id, | ||
| host: proxy.host, | ||
| port: proxy.port, | ||
| alive: false, | ||
| latencyMs, | ||
| error: err instanceof Error ? err.message : "Connection failed", | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
The variable alive is used on lines 36 and 37 but is never declared or defined in this scope, which will cause a runtime ReferenceError when a proxy test succeeds. Additionally, the timeout timer is not cleared if undiciFetch throws an error, leading to a potential resource leak. Wrapping the cleanup in a finally block and defining alive correctly resolves both issues.
async function testSingleProxy(proxy: { id: string; type: string; host: string; port: number }): Promise<TestResult> {
const proxyUrl = `${proxy.type}://${proxy.host}:${proxy.port}`;
const start = Date.now();
let timeout: ReturnType<typeof setTimeout> | undefined;
try {
const controller = new AbortController();
timeout = setTimeout(() => controller.abort(), TEST_TIMEOUT_MS);
const dispatcher = createProxyDispatcher(proxyUrl);
const resp = await undiciFetch(TEST_URL, {
method: "HEAD",
signal: controller.signal,
dispatcher,
headers: { "User-Agent": "OmniRoute/1.0" },
});
const latencyMs = Date.now() - start;
const alive = resp.status < 500;
await updateProxy(proxy.id, { status: alive ? "active" : "inactive" }).catch(() => {});
return { proxyId: proxy.id, host: proxy.host, port: proxy.port, alive, latencyMs };
} catch (err) {
const latencyMs = Date.now() - start;
await updateProxy(proxy.id, { status: "inactive" }).catch(() => {});
return {
proxyId: proxy.id,
host: proxy.host,
port: proxy.port,
alive: false,
latencyMs,
error: err instanceof Error ? err.message : "Connection failed",
};
} finally {
if (timeout) clearTimeout(timeout);
}
}| volumes: | ||
| - omniroute-prod-data:/app/data | ||
| - /home/openclaw/.omniroute:/app/data |
There was a problem hiding this comment.
Hardcoding a specific host path like /home/openclaw/.omniroute in the production docker-compose file breaks portability and will fail on other environments or for other users. It is highly recommended to use a relative path, an environment variable, or keep the named volume omniroute-prod-data for data persistence.
volumes:
- ${OMNIROUTE_DATA_DIR:-./data}:/app/data| */ | ||
|
|
||
| import { deleteProxyById, listProxies, updateProxy } from "@/lib/localDb"; | ||
| import { createProxyDispatcher } from "@omniroute/open-sse/utils/proxyDispatcher"; |
There was a problem hiding this comment.
Import clearDispatcherCache from @omniroute/open-sse/utils/proxyDispatcher so that stale dispatcher instances can be cleared from memory when proxies are auto-removed by the scheduler.
| import { createProxyDispatcher } from "@omniroute/open-sse/utils/proxyDispatcher"; | |
| import { createProxyDispatcher, clearDispatcherCache } from "@omniroute/open-sse/utils/proxyDispatcher"; |
| async function testOneProxy(proxy: { id: string; type: string; host: string; port: number }): Promise<boolean> { | ||
| const proxyUrl = `${proxy.type}://${proxy.host}:${proxy.port}`; | ||
| try { | ||
| const controller = new AbortController(); | ||
| const timeout = setTimeout(() => controller.abort(), TEST_TIMEOUT_MS); | ||
| const dispatcher = createProxyDispatcher(proxyUrl); | ||
| const resp = await undiciFetch(TEST_URL, { | ||
| method: "HEAD", | ||
| signal: controller.signal, | ||
| dispatcher, | ||
| headers: { "User-Agent": "OmniRoute/1.0" }, | ||
| }); | ||
| clearTimeout(timeout); | ||
| return resp.status < 500; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
The timeout timer is not cleared if undiciFetch throws an error, leading to a potential resource leak in the event loop. Wrapping the cleanup in a finally block ensures the timer is always cleared.
async function testOneProxy(proxy: { id: string; type: string; host: string; port: number }): Promise<boolean> {
const proxyUrl = `${proxy.type}://${proxy.host}:${proxy.port}`;
let timeout: ReturnType<typeof setTimeout> | undefined;
try {
const controller = new AbortController();
timeout = setTimeout(() => controller.abort(), TEST_TIMEOUT_MS);
const dispatcher = createProxyDispatcher(proxyUrl);
const resp = await undiciFetch(TEST_URL, {
method: "HEAD",
signal: controller.signal,
dispatcher,
headers: { "User-Agent": "OmniRoute/1.0" },
});
return resp.status < 500;
} catch {
return false;
} finally {
if (timeout) clearTimeout(timeout);
}
}|
|
||
| console.log( | ||
| `${LOG_PREFIX} Sweep complete: ${tested} tested, ${alive} alive, ${removed} auto-removed` | ||
| ); |
There was a problem hiding this comment.
When proxies are auto-removed by the scheduler, the proxy dispatcher cache is not cleared. This can leave stale dispatcher instances in memory. It is recommended to call clearDispatcherCache() if any proxies were removed.
if (removed > 0) {
try {
clearDispatcherCache();
} catch {
// non-critical
}
}
console.log(
`${LOG_PREFIX} Sweep complete: ${tested} tested, ${alive} alive, ${removed} auto-removed`
);| <span className="text-xs text-text-muted">{selectedIds.size} selected</span> | ||
| <Button | ||
| size="sm" | ||
| variant="secondary" | ||
| icon="delete" | ||
| onClick={handleBatchDelete} | ||
| loading={batchDeleting} | ||
| className="!text-red-400 !border-red-500/30" | ||
| data-testid="proxy-registry-batch-delete" | ||
| > | ||
| {`Delete ${selectedIds.size} selected`} |
There was a problem hiding this comment.
…267) Port upstream cheaper-inference#5918 (proxy batch/health). Adds a proxy-registry batch layer (ProxyBatchActions/ProxyCheckboxCell/ProxyStatusBadge/ProxyHealthCell/ ProxyBulkImportModal/useProxyBatchOperations) and two local-only routes: POST /api/settings/proxies/batch-delete (Zod ids 1..100 + force) and POST /api/settings/proxies/auto-test (HEAD-probe via connection dispatcher, optional autoRemove). Adds proxyHealth/scheduler.ts — an env-gated background sweep wired from instrumentation-node.ts, skipped during build and when OMNIROUTE_DISABLE_BACKGROUND_SERVICES is set. Deliberately drops upstream's model.ts transitive-alias change: it collapsed the fork's oc/ -> opencode routing distinction (fork cheaper-inference#2901), and the proxy feature does not depend on resolveProviderAlias. Upstream governance scaffolding (.cursorrules, _rules/*, AGENTS.md/README rewrites) excluded. Regression guard: tests/unit/proxy-batch-health-5918.test.ts (6 tests). Rebaselined ProxyRegistryManager.tsx file-size (1089->1117, own growth). Co-authored-by: Paijo <oyi77@users.noreply.github.com>
|
Thanks @oyi77 — there's real value here (proxy batch-delete, auto-test, health scheduler, alias-resolution fix). Three things before merge: (1) Scope creep — the branch includes a |
…lias fix Proxy-registry batch management: - POST /api/settings/proxies/batch-delete — delete many proxies at once - POST /api/settings/proxies/auto-test — reachability-test with optional auto-remove - background health scheduler (src/lib/proxyHealth/scheduler.ts) that periodically probes proxies and can auto-remove persistently-dead ones - dashboard batch-select checkboxes, Test All action, colored health indicators Alias fix: resolveProviderAlias() now follows the alias chain transitively (oc -> opencode -> opencode-zen) with depth-limit + seen-set cycle guards, instead of resolving only one hop to the intermediate provider. Security/hygiene over the original PR: - proxy health probe target is now operator-configurable (PROXY_HEALTH_TEST_URL) instead of a hardcoded endpoint - new PROXY_HEALTH_* / PROXY_AUTO_REMOVE* env vars documented in .env.example + docs/reference/ENVIRONMENT.md - dropped the unrelated scope-creep files (.cursorrules, AGENTS.md, _rules/*, docker-compose.prod.yml) from the original branch Regression guards: tests/unit/provider-alias-transitive-5918.test.ts, tests/unit/proxy-batch-routes-5918.test.ts. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
4645268 to
143361e
Compare
ad58bc1
into
cheaper-inference:release/v3.8.44
…bsorption - fix(models): stop resolveProviderAlias at registered provider ids so oc/ reaches the no-auth opencode provider again (#2901 contract, regressed by #5918's transitive chain; transitivity kept across alias-only hops) - fix(auggie): handle async EPIPE 'error' events on child stdin so a fast-exiting CLI surfaces a sanitized error instead of crashing (both spawn sites); deflakes auggie-executor tests - test: align provider family count 166->167 (Kenari #6104), regenerate translate-path golden on Linux (+kenari), opencode quota scope provider->connection (#6061) - quality(test-masking): add _deletedWithReplacement allowlist support to check-test-masking.mjs (deletion exempt ONLY when the declared replacement test exists in HEAD; 5 new gate unit tests) + reduction allowlist entries for the verified #5958/#6088/#5816 migrations + targetExhaustion-> combo-target-exhaustion replacement (#5976, 21 cases/52 asserts vs 13/37) - quality(file-size): absorb v3.8.44 cycle drift (oauth route 960, providerLimits 998, chat 1662, auth 2426) with justification; #6158 will restore the oauth-route freeze - changelog: bullets for the above + the #6155 cooling-panel build fix
- fix(dashboard): /dashboard/system/proxy 500'd on EVERY render — #5918 put useProxyBatchOperations(load) before the const load declaration (TDZ ReferenceError, digest 539380095). Hook block moved after load; SSR renderToString regression test added (the exact crash mode). - fix(server): TRACE/TRACK/CONNECT crashed Next's middleware adapter (undici cannot represent them) into a raw 500 on every route — the raw HTTP method guard now answers 405 + Allow up-front (dast-smoke Schemathesis finding on /api/keys/{id}/devices); guard test added. - fix(api): restore Zod validation on the provider-scoped chat route via a .passthrough() schema preserving #5907's relaxed semantics (t06 gate). - docs(openapi): /api/keys/{id}/devices 401 now refs the management error envelope (Schemathesis schema-conformance). - quality: rebaseline i18nUiCoverage 77.5->76.8 (+~1352 new en.json UI keys from the cycle await the async translation workflow; v3.8.39 precedent). - CodeQL: dismissed 2 incomplete-url-substring FPs on unit-test asserts (v3.8.35 precedent) with Hard Rule #14 justifications. - changelog: bullets for the above + 42 i18n mirrors re-synced
Rebuilds the proxy settings page (dashboard > system > proxy) around a new 7-tab structure (Overview, Global Config, Proxy Pool, Free Pool, Tier Control, Options, Documentation) so the 3-tier proxy control, scraper config, automation rules, and relay deployment all read as one coherent system instead of scattered ad-hoc additions. - Fix ProxyPoolTab, broken at HEAD since a merge kept a half-finished local version referencing undeclared state (menuOpen, vercelModalOpen, etc.); rebuilt from the complete upstream/main version (grouped Deploy Relay dropdown) plus the local pool-maintenance actions (Test All & Remove Bad, Import Best from Free Pool). - Add a shared SelectionToolbar with always-visible Select All / Deselect All and Delete Selected buttons (Delete disabled at zero selected); wire it into the Proxy Pool registry, Free Pool, and the new Tier Control tab. - Promote ThreeTierProxyControl out of Free Pool into its own tab, fully i18n'd, with tier lifecycle rules now visible as UI copy (previously only in code comments) and shared with a new Overview tab's tier-rules card. - Add OverviewTab: pool/registry/free-pool/tier stat cards, automation status, and tier-rule summary with deep links into the relevant tabs. - Restyle GlobalConfigTab, OptionsTab, and DocumentationTab into a consistent card layout; replace window.confirm/alert with ConfirmModal across the registry, free pool, and tier control bulk-delete flows. Extract ProxyRegistryManager's three modals into proxy/registry/ to keep it maintainable; the useProxyBatchOperations(load) ordering guard (cheaper-inference#5918 TDZ regression) is preserved. - Remove dead OneproxyTab (unreachable since /dashboard/system/1proxy became a redirect) and its i18n keys. - Add ~90 new i18n keys to en.json, synced to all 41 locales via i18n:sync-ui. Verified: full-repo tsc diff shows zero new errors and 13 resolved (the broken ProxyPoolTab's own errors); eslint reports zero new violations on touched files (all 4 remaining warnings pre-date this change, confirmed via git show main). New/updated component tests (SelectionToolbar, ProxyPoolTab incl. SSR smoke, ThreeTierProxyControl, OverviewTab, free-pool-tab) are written but currently cannot execute end-to-end: every UI test in this repo (new and pre-existing alike) hits a single unrelated, pre-existing duplicate-export bug in src/shared/constants/providers.ts from an earlier upstream merge, confirmed present on main and out of scope for this change.
… the per-test budget The remaining ui-shard reds were one class, not six bugs: every one of them did `await import(<heavy component>)` INSIDE an `it()`, so Vite's transform of the dependency graph was billed to that test's timeout. Measured costs against the budgets they had to fit in: ProxyRegistryManager 86s import vs 30s / 60s / 5s budgets (render itself: 567ms) claudeTlsClient ~12s import vs 5s default useProviderConnections 1050-line hook, whole dashboard graph, vs 5s default That is why they looked like cross-file pollution: on an idle box the import squeaked under the limit, and under the ui suite's 20 parallel workers it did not. Running claudeTlsClient ALONE on a loaded box reproduces it — the trigger is CPU contention, not a neighbouring file. The sibling chatgptTlsClient/grokTlsClient tests import the same graph and never fail, because they import statically at module scope, where the cost falls on the collection phase which has no per-test budget. Every fix here does the same: static import or a beforeAll with its own budget. AutoComboCatalog also explains its own blast radius: the timeout aborted inside an open act(), leaking an unbalanced act scope that then failed the file's three remaining tests in ~20ms with 'overlapping act() calls'. One slow import, four reds. CoolingConnectionsPanel is the one production change. It imported providerText from the ../providerPageHelpers barrel, but that symbol is DEFINED in the ../providerCredentialText leaf and only re-exported by the barrel — which drags providerRegistry (352 providers) and the rest of the provider-page graph into a "use client" component for one string helper. Verified before accepting: the component used nothing else from the barrel, the barrel has no top-level side-effect to lose (the empty-registry hazard this repo has hit before does not apply), typecheck:core is clean, and the panel's first test drops from ~4s to 95ms. The import was suboptimal, never broken — the screen was not failing for users. No assertion was weakened anywhere. expect() counts are unchanged (25/25, 4/4) or up by one (AutoComboCatalog 11 -> 12); the #8855 autofill sentinels, the data-1p-ignore / data-lpignore guards and the dead-status round-trip are intact. The #5918 TDZ guard was proven still live by mutation, not by absence of red: moving useProxyBatchOperations(load) above its const reproduced 'ReferenceError: Cannot access load before initialization' in 207ms, then the production file was restored (diff empty). tests/unit/ui under load: 17 failed files / 45 failed tests -> 4 failed files / 4 failed tests, none of them these. The four left are compression-guidance-7530, compressionPanel, compressionUltraTier and lobe-provider-icons-stepfun, untouched and uninvestigated. Refs #10692
…lias fix (cheaper-inference#5918) Proxy-registry batch management (batch-delete, auto-test, background health scheduler) + fix resolveProviderAlias to follow the alias chain transitively (oc -> opencode -> opencode-zen). Probe target now operator-configurable via PROXY_HEALTH_TEST_URL. Scope-creep files from the original branch dropped. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
…bsorption - fix(models): stop resolveProviderAlias at registered provider ids so oc/ reaches the no-auth opencode provider again (cheaper-inference#2901 contract, regressed by cheaper-inference#5918's transitive chain; transitivity kept across alias-only hops) - fix(auggie): handle async EPIPE 'error' events on child stdin so a fast-exiting CLI surfaces a sanitized error instead of crashing (both spawn sites); deflakes auggie-executor tests - test: align provider family count 166->167 (Kenari cheaper-inference#6104), regenerate translate-path golden on Linux (+kenari), opencode quota scope provider->connection (cheaper-inference#6061) - quality(test-masking): add _deletedWithReplacement allowlist support to check-test-masking.mjs (deletion exempt ONLY when the declared replacement test exists in HEAD; 5 new gate unit tests) + reduction allowlist entries for the verified cheaper-inference#5958/cheaper-inference#6088/cheaper-inference#5816 migrations + targetExhaustion-> combo-target-exhaustion replacement (cheaper-inference#5976, 21 cases/52 asserts vs 13/37) - quality(file-size): absorb v3.8.44 cycle drift (oauth route 960, providerLimits 998, chat 1662, auth 2426) with justification; cheaper-inference#6158 will restore the oauth-route freeze - changelog: bullets for the above + the cheaper-inference#6155 cooling-panel build fix
- fix(dashboard): /dashboard/system/proxy 500'd on EVERY render — cheaper-inference#5918 put useProxyBatchOperations(load) before the const load declaration (TDZ ReferenceError, digest 539380095). Hook block moved after load; SSR renderToString regression test added (the exact crash mode). - fix(server): TRACE/TRACK/CONNECT crashed Next's middleware adapter (undici cannot represent them) into a raw 500 on every route — the raw HTTP method guard now answers 405 + Allow up-front (dast-smoke Schemathesis finding on /api/keys/{id}/devices); guard test added. - fix(api): restore Zod validation on the provider-scoped chat route via a .passthrough() schema preserving cheaper-inference#5907's relaxed semantics (t06 gate). - docs(openapi): /api/keys/{id}/devices 401 now refs the management error envelope (Schemathesis schema-conformance). - quality: rebaseline i18nUiCoverage 77.5->76.8 (+~1352 new en.json UI keys from the cycle await the async translation workflow; v3.8.39 precedent). - CodeQL: dismissed 2 incomplete-url-substring FPs on unit-test asserts (v3.8.35 precedent) with Hard Rule cheaper-inference#14 justifications. - changelog: bullets for the above + 42 i18n mirrors re-synced
… the per-test budget The remaining ui-shard reds were one class, not six bugs: every one of them did `await import(<heavy component>)` INSIDE an `it()`, so Vite's transform of the dependency graph was billed to that test's timeout. Measured costs against the budgets they had to fit in: ProxyRegistryManager 86s import vs 30s / 60s / 5s budgets (render itself: 567ms) claudeTlsClient ~12s import vs 5s default useProviderConnections 1050-line hook, whole dashboard graph, vs 5s default That is why they looked like cross-file pollution: on an idle box the import squeaked under the limit, and under the ui suite's 20 parallel workers it did not. Running claudeTlsClient ALONE on a loaded box reproduces it — the trigger is CPU contention, not a neighbouring file. The sibling chatgptTlsClient/grokTlsClient tests import the same graph and never fail, because they import statically at module scope, where the cost falls on the collection phase which has no per-test budget. Every fix here does the same: static import or a beforeAll with its own budget. AutoComboCatalog also explains its own blast radius: the timeout aborted inside an open act(), leaking an unbalanced act scope that then failed the file's three remaining tests in ~20ms with 'overlapping act() calls'. One slow import, four reds. CoolingConnectionsPanel is the one production change. It imported providerText from the ../providerPageHelpers barrel, but that symbol is DEFINED in the ../providerCredentialText leaf and only re-exported by the barrel — which drags providerRegistry (352 providers) and the rest of the provider-page graph into a "use client" component for one string helper. Verified before accepting: the component used nothing else from the barrel, the barrel has no top-level side-effect to lose (the empty-registry hazard this repo has hit before does not apply), typecheck:core is clean, and the panel's first test drops from ~4s to 95ms. The import was suboptimal, never broken — the screen was not failing for users. No assertion was weakened anywhere. expect() counts are unchanged (25/25, 4/4) or up by one (AutoComboCatalog 11 -> 12); the cheaper-inference#8855 autofill sentinels, the data-1p-ignore / data-lpignore guards and the dead-status round-trip are intact. The cheaper-inference#5918 TDZ guard was proven still live by mutation, not by absence of red: moving useProxyBatchOperations(load) above its const reproduced 'ReferenceError: Cannot access load before initialization' in 207ms, then the production file was restored (diff empty). tests/unit/ui under load: 17 failed files / 45 failed tests -> 4 failed files / 4 failed tests, none of them these. The four left are compression-guidance-7530, compressionPanel, compressionUltraTier and lobe-provider-icons-stepfun, untouched and uninvestigated. Refs cheaper-inference#10692
Summary
Proxy Management
Bug Fix
Infrastructure
CI Status
Testing
Breaking Changes
None. All changes are additive.