From 5615f233dfb7e6d1af11035272a4da38be6e6cb4 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 26 Jun 2026 20:03:23 -0300 Subject: [PATCH 1/2] fix(db): dedupe web-session cookie credentials on connection create (#3368) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bulk web-session import (#3368 PR6) created a duplicate connection every time the same cookie/token was re-imported under a different name, because createProviderConnection only deduped oauth and apikey auth types. Add a cookie branch that upserts by name (parity with apikey) and by credential value, so re-importing a session updates the existing connection instead of inserting a duplicate. provider_specific_data is plaintext JSON, so the value is compared directly without decryption. The pure credential-key + JSON-parse helpers are extracted to a new leaf module src/lib/db/webSessionDedup.ts (unit-tested, allowlisted INTENTIONALLY_INTERNAL in check-db-rules since it is consumed only by providers.ts — Hard Rule #2); the cookie SQL queries + match loop stay as cohesive DB wiring next to the oauth/ apikey branches. providers.ts 1063->1093 rebaselined with justification (structural shrink tracked in #3501). --- config/quality/file-size-baseline.json | 3 +- scripts/check/check-db-rules.mjs | 1 + src/lib/db/providers.ts | 30 ++++ src/lib/db/webSessionDedup.ts | 57 ++++++++ .../db-provider-cookie-dedup-3368.test.ts | 132 ++++++++++++++++++ tests/unit/web-session-dedup-3368.test.ts | 42 ++++++ 6 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 src/lib/db/webSessionDedup.ts create mode 100644 tests/unit/db-provider-cookie-dedup-3368.test.ts create mode 100644 tests/unit/web-session-dedup-3368.test.ts diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index 18ae9c2a3b4..043f3ceba46 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -13,6 +13,7 @@ "_rebaseline_2026_06_24_combo_cooldown_wait_quota_share": "Feature quota-share combo cooldown-aware retry (Variante A) own growth: open-sse/services/combo.ts 3225->3293 (+68 = the cooldown-wait wrap inside handleComboChat's quota-share path. The existing setTry loop body is LEFT at its original indentation: instead of an outer `while (true)` (which would re-indent ~1600 lines and bloat the review), the setTry loop is hoisted into a small recursive closure `dispatchWithCooldownRetry`, and a wait+redispatch is a tail `return dispatchWithCooldownRetry()` — re-running ONLY the set loop (exactly the prior continue-to-top-of-set-loop semantics) while selection/shadow-routing/setup above stay untouched. At the 429 crystallization point the lock reason is resolved via getModelLockoutInfo, the decision via the new pure resolveComboCooldownWaitDecision, then await waitForCooldownAwareRetry (499 on abort), decrement the budget, recurse. Gated to strategy==='quota-share' && comboCooldownWait.enabled. `git diff` == `git diff -w` for combo.ts (zero re-indentation noise). The gating policy + reason resolution are extracted to the new pure leaf open-sse/services/combo/comboCooldownRetry.ts (1098 (+115 = the ComboCooldownWaitCard exposing enabled/maxWaitMs/maxAttempts/budgetMs in Settings > Resilience, mirroring WaitForCooldownCard; wired through GET/PATCH in src/app/api/resilience/route.ts + comboCooldownWaitSettingsSchema in src/shared/validation/schemas/settings.ts; new UI labels use the t(key)||English-fallback pattern, en-only). Structural shrink of combo.ts + ResilienceTab tracked in #3501.", "_rebaseline_2026_06_24_quota_share_concurrency_limit": "Feature FASE 2.1 (per-connection concurrency limit for quota-share combos) own growth. The quota-share gating in selectQuotaShareTarget is FAIL-OPEN (an at-cap connection is only deprioritized, never hard-blocked), so with a single-connection subscription-account pool concurrent requests still flood the account — empirically proven on the .15 deploy: 3 concurrent share-key calls to one minimax connection with max_concurrent=1 were all dispatched within 94ms. This adds a per-CONNECTION semaphore around the quota-share dispatch so excess concurrent requests WAIT in the queue instead of flooding (key = qsconn:, cap = the connection's max_concurrent; fail-open on a saturated queue/timeout to never worsen availability). open-sse/services/combo.ts 3306->3340 (+34 = the irreducible chokepoint wiring: the quotaShareConcurrencyEnabled gate, the acquire (lookupPositiveCap + acquireQuotaShareConcurrencySlot) around dispatchWithCooldownRetry, the release in the outer finally, and the updated cooldown-wait comment). ALL extractable logic lives in the new pure leaf open-sse/services/combo/quotaShareConcurrency.ts (841 (+41 = QuotaShareConcurrencyLimitSettings interface + default {enabled:true} + normalizeQuotaShareConcurrencyLimitSettings + resolve/merge/legacy-fallback wiring; crossed the 800 new-file cap, mirrors the existing comboCooldownWait block). src/app/(dashboard)/dashboard/settings/components/ResilienceTab.tsx 1098->1183 (+85 = QuotaShareConcurrencyLimitCard, a kill-switch toggle mirroring ComboCooldownWaitCard; wired through GET/PATCH in src/app/api/resilience/route.ts + quotaShareConcurrencyLimitSettingsSchema in src/shared/validation/schemas/settings.ts; t(key)||English-fallback, en-only). Covered by tests/unit/combo/quota-share-concurrency.test.ts + tests/unit/resilience-settings-quota-share-concurrency.test.ts. Structural shrink of combo.ts + ResilienceTab tracked in #3501.", "_rebaseline_2026_06_23_4774_combo_legacy_strip": "PR #4774 (KooshaPari, #4382 round-trip) own growth: src/app/(dashboard)/dashboard/combos/page.tsx 4434->4456 (+22 = the client-side LEGACY_COMBO_RESILIENCE_KEYS Set gains queueTimeoutMs + the 12 v3.8.31-era removed keys (queueDepth/fallbackDelayMs/handoffProviders/maxComboDepth/manifestRouting/complexityAwareRouting/pipeline_enabled/pipelineConcurrency/shadowRouting/evalRouting/resetAwareEnabled/resetAwareWindow) with explanatory comments, mirroring the server-side strip list in src/app/api/combos/[id]/route.ts so the modal never re-introduces removed keys on Save). Functional strip list, not a movable block; combos/page.tsx structural shrink tracked in #3501. Covered by tests/unit/combo-config.test.ts (auto-promote + passthrough + legacy-key round-trip).", + "_rebaseline_2026_06_26_3368_cookie_dedup": "Issue #3368 PR6 own growth: src/lib/db/providers.ts 1063->1093 (+30 = the cookie-auth dedup branch in createProviderConnection — name-based upsert + credential-value match, mirroring the existing oauth/apikey dedup so bulk web-session import stops creating duplicate connections on re-import). The pure credential-key + JSON-parse helpers (webSessionCredentialKey, parseProviderSpecificData) were extracted to the new src/lib/db/webSessionDedup.ts (1063 (+13 = resolveProviderNodeForConnection at the existing provider-node lookup — resolves a connection node by exact id OR the bare derived type when unambiguous, + import). Pure selection logic in the new src/lib/db/providerNodeSelect.ts (2289 (+10): #4530 só fiou maxCooldownMs nos 3 sites de combo.ts; os 4 sites de markAccountUnavailable (per-model quota, grok-web 403, per-model 403, local 404) nunca passavam o cap → resolvo mlSettings uma vez e passo maxCooldownMs em todos. tests/unit/db-core-init.test.ts 864->867 (+3): comentário explicando o cap intencional busy_timeout 5s->2s do v3.8.32. Wiring necessário ao chokepoint de lockout; não extraível. Coberto por model-lockout-max-cooldown.test.ts + db-core-init.test.ts. (Demais base-reds — áudio mp3 #912/#913 dedup de handler em geminiHelper.ts, e closure #3578 src/models/ no package.json files — não cresceram arquivo congelado.)", "_rebaseline_2026_06_21_v3833_cycle_open_latent_filesize": "Abertura do ciclo v3.8.33: 4 arquivos cresceram no ciclo v3.8.32 sem bump de baseline e o drift escapou do fast-path do release (check:file-size não roda nas fast-gates p/ release/*, só no PR→main full CI, e o crescimento veio de commits entre fca66c644 e o head do merge 912239f46 — ex. #4475 targetFormat). Medido em origin/main (idêntico, cherry-picks deste ciclo NÃO tocam estes 4): open-sse/services/usage.ts 3408->3414, src/lib/db/core.ts 1820->1825, src/lib/usage/providerLimits.ts 949->950, src/shared/constants/providers.ts 3242->3243. Reconcílio ao valor real de main p/ abrir o .33 verde; shrink estrutural rastreado em #3501.", @@ -211,7 +212,7 @@ "src/lib/db/core.ts": 1825, "src/lib/db/migrationRunner.ts": 1125, "src/lib/db/models.ts": 1259, - "src/lib/db/providers.ts": 1063, + "src/lib/db/providers.ts": 1093, "src/lib/db/proxies.ts": 1060, "src/lib/db/settings.ts": 1155, "src/lib/db/usageAnalytics.ts": 925, diff --git a/scripts/check/check-db-rules.mjs b/scripts/check/check-db-rules.mjs index e948bd90e4f..6bcfe69c25b 100644 --- a/scripts/check/check-db-rules.mjs +++ b/scripts/check/check-db-rules.mjs @@ -70,6 +70,7 @@ export const INTENTIONALLY_INTERNAL = new Set([ "stateReset", // db-internal: 3 callers dentro de src/lib/db/ (core, backup, apiKeys) para coordenação de reset "stats", // intentionally-internal: src/app/api/settings/database/refresh-stats/route.ts "tierConfig", // intentionally-internal: open-sse/services/tierResolver.ts (require() dinâmico) + "webSessionDedup", // db-internal: importado só por db/providers.ts (webSessionCredentialKey/parseProviderSpecificData — helpers puros de dedup de credencial web-session split do providers.ts, #3368 PR6) ]); // Alias para retrocompatibilidade com os testes existentes que importam KNOWN_UNEXPORTED. diff --git a/src/lib/db/providers.ts b/src/lib/db/providers.ts index c14a4a3e6eb..9f1a89bce5b 100644 --- a/src/lib/db/providers.ts +++ b/src/lib/db/providers.ts @@ -14,6 +14,7 @@ import { import { invalidateDbCache } from "./readCache"; import { normalizeProviderSpecificData } from "@/lib/providers/requestDefaults"; import { bumpProxyConfigGeneration } from "./settings"; +import { webSessionCredentialKey, parseProviderSpecificData } from "./webSessionDedup"; type JsonRecord = Record; @@ -268,6 +269,35 @@ export async function createProviderConnection(data: JsonRecord) { } } } + } else if (data.authType === "cookie") { + // #3368 PR6 — dedup web-session cookie/token credentials. Re-importing the + // same session (e.g. via bulk web-session import) under a different or blank + // name must update the existing connection instead of inserting a duplicate, + // mirroring the apikey dedup (#3023). + // 1) Name-based upsert for parity with the apikey path. + if (data.name) { + existing = + (db + .prepare( + "SELECT * FROM provider_connections WHERE provider = ? AND auth_type = 'cookie' AND name = ?" + ) + .get(data.provider, data.name) as JsonRecord | undefined) || null; + } + // 2) Credential-value dedup: provider_specific_data is plaintext JSON, so the + // stored cookie/token value can be compared directly (no decryption needed). + const newCredKey = webSessionCredentialKey(normalizedProviderSpecificData); + if (!existing && newCredKey) { + const cookieRows = db + .prepare("SELECT * FROM provider_connections WHERE provider = ? AND auth_type = 'cookie'") + .all(data.provider) as JsonRecord[]; + for (const row of cookieRows) { + const psd = parseProviderSpecificData(row.provider_specific_data); + if (psd && webSessionCredentialKey(psd) === newCredKey) { + existing = row; + break; + } + } + } } if (existing) { diff --git a/src/lib/db/webSessionDedup.ts b/src/lib/db/webSessionDedup.ts new file mode 100644 index 00000000000..341afc9a2d8 --- /dev/null +++ b/src/lib/db/webSessionDedup.ts @@ -0,0 +1,57 @@ +/** + * db/webSessionDedup.ts — pure helpers for de-duplicating web-session + * (cookie/token) provider credentials. Extracted from providers.ts so the + * cookie-dedup wiring there stays thin (#3368 PR6). No DB access here. + */ + +/** + * Reduce a `provider_specific_data` record to a single comparable credential + * value. Cookie/token credentials are mirrored across a provider's storage + * keys (e.g. `cookie`, `sessionToken`, `token`) with the same secret value, so + * any one of them identifies the session. Returns the trimmed value, or null + * when no usable string credential is present. + */ +const PREFERRED_CREDENTIAL_KEYS = [ + "cookie", + "token", + "sessionToken", + "session-token", + "sso", + "access_token", + "accessToken", +]; + +/** First trimmed non-empty string value among `keys` of `rec`, else null. */ +function firstNonEmptyString(rec: Record, keys: readonly string[]): string | null { + for (const key of keys) { + const value = rec[key]; + if (typeof value === "string" && value.trim()) return value.trim(); + } + return null; +} + +export function webSessionCredentialKey(psd: unknown): string | null { + if (!psd || typeof psd !== "object") return null; + const rec = psd as Record; + // Prefer canonical credential keys, then fall back to the first non-empty + // string value (sorted for determinism). + return ( + firstNonEmptyString(rec, PREFERRED_CREDENTIAL_KEYS) ?? + firstNonEmptyString(rec, Object.keys(rec).sort()) + ); +} + +/** Parse a stored `provider_specific_data` column (JSON string or object). */ +export function parseProviderSpecificData(raw: unknown): Record | null { + if (!raw) return null; + if (typeof raw === "object") return raw as Record; + if (typeof raw === "string") { + try { + const parsed = JSON.parse(raw); + return parsed && typeof parsed === "object" ? (parsed as Record) : null; + } catch { + return null; + } + } + return null; +} diff --git a/tests/unit/db-provider-cookie-dedup-3368.test.ts b/tests/unit/db-provider-cookie-dedup-3368.test.ts new file mode 100644 index 00000000000..9d5916a4ff1 --- /dev/null +++ b/tests/unit/db-provider-cookie-dedup-3368.test.ts @@ -0,0 +1,132 @@ +// #3368 PR6 — web-session (cookie) bulk import must dedupe credentials. +// Re-importing the same cookie/token (even under a different name) must UPDATE +// the existing connection, not insert a duplicate row — mirroring the apikey +// dedup behavior (#3023). This test fails before the cookie-dedup branch exists. +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"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cookie-dedup-")); +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"); + +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: unknown) { + const code = (error as { code?: string } | null)?.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.beforeEach(async () => { + await resetStorage(); +}); + +test.after(async () => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("#3368 cookie dedup: re-importing the same cookie under a different name updates, not duplicates", async () => { + await providersDb.createProviderConnection({ + provider: "qwen-ai", + authType: "cookie", + name: "Import A", + apiKey: null, + providerSpecificData: { cookie: "session=SAME_COOKIE_VALUE" }, + isActive: true, + }); + await providersDb.createProviderConnection({ + provider: "qwen-ai", + authType: "cookie", + name: "Import B (same cookie)", + apiKey: null, + providerSpecificData: { cookie: "session=SAME_COOKIE_VALUE" }, + isActive: true, + }); + + const conns = await providersDb.getProviderConnections({ provider: "qwen-ai" }); + assert.equal(conns.length, 1, "same cookie value must dedupe to a single connection"); +}); + +test("#3368 cookie dedup: a different cookie creates a separate connection", async () => { + await providersDb.createProviderConnection({ + provider: "qwen-ai", + authType: "cookie", + name: "Account 1", + apiKey: null, + providerSpecificData: { cookie: "session=COOKIE_ONE" }, + isActive: true, + }); + await providersDb.createProviderConnection({ + provider: "qwen-ai", + authType: "cookie", + name: "Account 2", + apiKey: null, + providerSpecificData: { cookie: "session=COOKIE_TWO" }, + isActive: true, + }); + + const conns = await providersDb.getProviderConnections({ provider: "qwen-ai" }); + assert.equal(conns.length, 2, "distinct cookies must remain distinct connections"); +}); + +test("#3368 cookie dedup: token-kind credential (no `cookie` key) also dedupes by value", async () => { + await providersDb.createProviderConnection({ + provider: "kilo-code", + authType: "cookie", + name: "Token A", + apiKey: null, + providerSpecificData: { token: "TOK_123", userToken: "TOK_123" }, + isActive: true, + }); + await providersDb.createProviderConnection({ + provider: "kilo-code", + authType: "cookie", + name: "Token A re-import", + apiKey: null, + providerSpecificData: { token: "TOK_123", userToken: "TOK_123" }, + isActive: true, + }); + + const conns = await providersDb.getProviderConnections({ provider: "kilo-code" }); + assert.equal(conns.length, 1, "same token value must dedupe even without a cookie key"); +}); + +test("#3368 cookie dedup: name-based upsert updates the same-named cookie connection", async () => { + await providersDb.createProviderConnection({ + provider: "qwen-ai", + authType: "cookie", + name: "Stable Name", + apiKey: null, + providerSpecificData: { cookie: "session=FIRST" }, + isActive: true, + }); + await providersDb.createProviderConnection({ + provider: "qwen-ai", + authType: "cookie", + name: "Stable Name", + apiKey: null, + providerSpecificData: { cookie: "session=ROTATED" }, + isActive: true, + }); + + const conns = await providersDb.getProviderConnections({ provider: "qwen-ai" }); + assert.equal(conns.length, 1, "same provider+name must upsert, not duplicate"); +}); diff --git a/tests/unit/web-session-dedup-3368.test.ts b/tests/unit/web-session-dedup-3368.test.ts new file mode 100644 index 00000000000..12d3fb69aa2 --- /dev/null +++ b/tests/unit/web-session-dedup-3368.test.ts @@ -0,0 +1,42 @@ +// #3368 PR6 — unit coverage for the pure web-session dedup helpers extracted +// from providers.ts (src/lib/db/webSessionDedup.ts). +import test from "node:test"; +import assert from "node:assert/strict"; + +const { webSessionCredentialKey, parseProviderSpecificData } = await import( + "../../src/lib/db/webSessionDedup.ts" +); + +test("webSessionCredentialKey prefers the cookie field", () => { + assert.equal( + webSessionCredentialKey({ cookie: "session=ABC", note: "x" }), + "session=ABC" + ); +}); + +test("webSessionCredentialKey falls back to token-kind keys", () => { + assert.equal(webSessionCredentialKey({ token: "TOK", userToken: "TOK" }), "TOK"); +}); + +test("webSessionCredentialKey trims and ignores empty/non-string values", () => { + assert.equal(webSessionCredentialKey({ cookie: " v=1 " }), "v=1"); + assert.equal(webSessionCredentialKey({ cookie: " ", sso: "real" }), "real"); + assert.equal(webSessionCredentialKey({}), null); + assert.equal(webSessionCredentialKey(null), null); + assert.equal(webSessionCredentialKey("not-an-object"), null); +}); + +test("webSessionCredentialKey is deterministic for arbitrary keys", () => { + const a = webSessionCredentialKey({ zeta: "z", alpha: "a" }); + const b = webSessionCredentialKey({ alpha: "a", zeta: "z" }); + assert.equal(a, b); + assert.equal(a, "a"); // sorted key order → "alpha" wins +}); + +test("parseProviderSpecificData handles JSON strings, objects, and junk", () => { + assert.deepEqual(parseProviderSpecificData('{"cookie":"x"}'), { cookie: "x" }); + assert.deepEqual(parseProviderSpecificData({ cookie: "x" }), { cookie: "x" }); + assert.equal(parseProviderSpecificData("not json"), null); + assert.equal(parseProviderSpecificData(null), null); + assert.equal(parseProviderSpecificData(""), null); +}); From a105e8914cc516302a99f9e1b1fc068808ef7056 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 26 Jun 2026 20:03:23 -0300 Subject: [PATCH 2/2] feat(mcp): add browser-pool observability tool + lifecycle metrics (#3368) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit observability — getBrowserPoolStatus() existed but had zero callers. Add cumulative lifecycle counters (launches/failures, context create/reuse/evict/ release, create failures, shutdowns + last reason) exposed via getBrowserPoolMetrics(), and surface them through a new read:health MCP tool omniroute_browser_pool_status (auto-registered via the poolTools loop). The per-context pending-cleanup is factored into settlePendingContext so acquireBrowserContext stays under the function-length ceiling. Also fixes a latent proxy-options type error surfaced once browserPool.ts is pulled into typecheck-core scope by the new tool import. --- open-sse/mcp-server/tools/poolTools.ts | 19 +++++++ open-sse/services/browserPool.ts | 26 +++++++--- src/lib/db/providers.ts | 70 +++++++++++++++----------- src/shared/constants/mcpScopes.ts | 2 + tests/unit/mcp-pool-tools-3368.test.ts | 59 ++++++++++++++++++++-- 5 files changed, 137 insertions(+), 39 deletions(-) diff --git a/open-sse/mcp-server/tools/poolTools.ts b/open-sse/mcp-server/tools/poolTools.ts index 4437e68f88f..7c217db8e81 100644 --- a/open-sse/mcp-server/tools/poolTools.ts +++ b/open-sse/mcp-server/tools/poolTools.ts @@ -12,6 +12,7 @@ import { z } from "zod"; import { PoolRegistry } from "../../services/sessionPool/poolRegistry.ts"; import { getWebSessionPoolHealth } from "../../services/webSessionPoolHealth.ts"; +import { getBrowserPoolMetrics } from "../../services/browserPool.ts"; // ─── Input Schemas ───────────────────────────────────────────────────────── @@ -140,6 +141,16 @@ export async function handlePoolHealth( return report as unknown as Record; } +export const browserPoolStatusInput = z.object({}); + +/** + * Handle browser_pool_status tool (#3368 PR7): return the stealth browser + * pool's live status plus cumulative lifecycle telemetry. + */ +export async function handleBrowserPoolStatus(): Promise> { + return getBrowserPoolMetrics(); +} + // ─── Tool Registry ───────────────────────────────────────────────────────── export const poolTools = { @@ -183,4 +194,12 @@ export const poolTools = { inputSchema: poolHealthInput, handler: (args: z.infer) => handlePoolHealth(args), }, + omniroute_browser_pool_status: { + name: "omniroute_browser_pool_status", + description: + "Returns the stealth browser pool's live status (enabled, active contexts, browser running, stealth available, idle age) plus cumulative lifecycle telemetry: browser launches/failures, context create/reuse/evict/release counts, context-create failures, and shutdowns with the last reason.", + scopes: ["read:health"], + inputSchema: browserPoolStatusInput, + handler: () => handleBrowserPoolStatus(), + }, }; diff --git a/open-sse/services/browserPool.ts b/open-sse/services/browserPool.ts index 24ff06bb737..5343205cea1 100644 --- a/open-sse/services/browserPool.ts +++ b/open-sse/services/browserPool.ts @@ -197,10 +197,17 @@ export async function resolvePlaywrightProxy( const p = await resolver(providerKey); if (!p?.host) return undefined; const scheme = p.type === "socks5" ? "socks5" : "http"; - return { + // Build explicitly instead of a conditional object spread: the spread form + // widens username/password to `{}` under the LaunchOptions["proxy"] type, + // tripping typecheck once browserPool.ts is pulled into typecheck-core scope. + const proxy: NonNullable = { server: `${scheme}://${p.host}:${p.port}`, - ...(p.username ? { username: p.username, password: p.password ?? "" } : {}), }; + if (p.username) { + proxy.username = String(p.username); + proxy.password = p.password == null ? "" : String(p.password); + } + return proxy; } catch (err) { console.warn("[BrowserPool] Failed to resolve proxy from DB:", err); return undefined; @@ -291,6 +298,14 @@ function parseCookieString( }>; } +// Clear a key from the pending-creation map once its promise settles, counting +// failures. Kept as a leaf helper so acquireBrowserContext stays under the +// function-length ceiling (#3368 PR7 metrics). +function settlePendingContext(key: string, failed: boolean): void { + if (failed) state.metrics.contextCreateFailures++; + state.pendingContexts.delete(key); +} + export async function acquireBrowserContext( key: string, options: BrowserPoolContextOptions @@ -382,11 +397,8 @@ export async function acquireBrowserContext( state.pendingContexts.set(key, createPromise); createPromise - .then(() => state.pendingContexts.delete(key)) - .catch(() => { - state.metrics.contextCreateFailures++; - state.pendingContexts.delete(key); - }); + .then(() => settlePendingContext(key, false)) + .catch(() => settlePendingContext(key, true)); return createPromise; } diff --git a/src/lib/db/providers.ts b/src/lib/db/providers.ts index 9f1a89bce5b..f6e14153d10 100644 --- a/src/lib/db/providers.ts +++ b/src/lib/db/providers.ts @@ -192,6 +192,42 @@ export async function getProviderConnectionById(id: string) { ); } +// #3368 PR6 — dedup web-session cookie/token credentials on connection create. +// Re-importing the same session (e.g. via bulk web-session import) under a +// different or blank name must update the existing connection instead of +// inserting a duplicate, mirroring the apikey dedup (#3023). Extracted from +// createProviderConnection to keep that function below the complexity baseline. +// provider_specific_data is plaintext JSON, so the value is compared directly +// without decryption. +function findExistingCookieConnection( + db: DbLike, + provider: unknown, + name: unknown, + normalizedProviderSpecificData: unknown +): JsonRecord | null { + // 1) Name-based upsert for parity with the apikey path. + if (name) { + const byName = + (db + .prepare( + "SELECT * FROM provider_connections WHERE provider = ? AND auth_type = 'cookie' AND name = ?" + ) + .get(provider, name) as JsonRecord | undefined) || null; + if (byName) return byName; + } + // 2) Credential-value dedup against existing cookie rows. + const newCredKey = webSessionCredentialKey(normalizedProviderSpecificData); + if (!newCredKey) return null; + const cookieRows = db + .prepare("SELECT * FROM provider_connections WHERE provider = ? AND auth_type = 'cookie'") + .all(provider) as JsonRecord[]; + for (const row of cookieRows) { + const psd = parseProviderSpecificData(row.provider_specific_data); + if (psd && webSessionCredentialKey(psd) === newCredKey) return row; + } + return null; +} + export async function createProviderConnection(data: JsonRecord) { const db = getDbInstance() as unknown as DbLike; const now = new Date().toISOString(); @@ -270,34 +306,12 @@ export async function createProviderConnection(data: JsonRecord) { } } } else if (data.authType === "cookie") { - // #3368 PR6 — dedup web-session cookie/token credentials. Re-importing the - // same session (e.g. via bulk web-session import) under a different or blank - // name must update the existing connection instead of inserting a duplicate, - // mirroring the apikey dedup (#3023). - // 1) Name-based upsert for parity with the apikey path. - if (data.name) { - existing = - (db - .prepare( - "SELECT * FROM provider_connections WHERE provider = ? AND auth_type = 'cookie' AND name = ?" - ) - .get(data.provider, data.name) as JsonRecord | undefined) || null; - } - // 2) Credential-value dedup: provider_specific_data is plaintext JSON, so the - // stored cookie/token value can be compared directly (no decryption needed). - const newCredKey = webSessionCredentialKey(normalizedProviderSpecificData); - if (!existing && newCredKey) { - const cookieRows = db - .prepare("SELECT * FROM provider_connections WHERE provider = ? AND auth_type = 'cookie'") - .all(data.provider) as JsonRecord[]; - for (const row of cookieRows) { - const psd = parseProviderSpecificData(row.provider_specific_data); - if (psd && webSessionCredentialKey(psd) === newCredKey) { - existing = row; - break; - } - } - } + existing = findExistingCookieConnection( + db, + data.provider, + data.name, + normalizedProviderSpecificData + ); } if (existing) { diff --git a/src/shared/constants/mcpScopes.ts b/src/shared/constants/mcpScopes.ts index 78463e51fc6..80f987c6481 100644 --- a/src/shared/constants/mcpScopes.ts +++ b/src/shared/constants/mcpScopes.ts @@ -73,6 +73,8 @@ export const MCP_TOOL_SCOPES: Record = { omniroute_pool_health: ["read:health"], omniroute_pool_reset: ["write:resilience"], omniroute_pool_warm: ["write:resilience"], + // Stealth browser pool observability (#3368 PR7) + omniroute_browser_pool_status: ["read:health"], } as const; // ============ Scope Groups ============ diff --git a/tests/unit/mcp-pool-tools-3368.test.ts b/tests/unit/mcp-pool-tools-3368.test.ts index 0041eb0b807..5994cab3aa6 100644 --- a/tests/unit/mcp-pool-tools-3368.test.ts +++ b/tests/unit/mcp-pool-tools-3368.test.ts @@ -10,9 +10,13 @@ import { readFileSync } from "node:fs"; // the observability tools cannot silently fall out of the live MCP server again. const { poolTools } = await import("../../open-sse/mcp-server/tools/poolTools.ts"); -const { handlePoolStatus, handlePoolSessions, handlePoolReset, handlePoolWarm } = await import( - "../../open-sse/mcp-server/tools/poolTools.ts" -); +const { + handlePoolStatus, + handlePoolSessions, + handlePoolReset, + handlePoolWarm, + handleBrowserPoolStatus, +} = await import("../../open-sse/mcp-server/tools/poolTools.ts"); const { MCP_TOOL_SCOPES, MCP_SCOPE_LIST } = await import( "../../src/shared/constants/mcpScopes.ts" ); @@ -23,6 +27,8 @@ const POOL_TOOL_NAMES = [ "omniroute_pool_reset", "omniroute_pool_warm", "omniroute_pool_health", + // #3368 PR7 — stealth browser pool observability + "omniroute_browser_pool_status", ]; const serverSource = readFileSync( @@ -60,7 +66,7 @@ test("server.ts reserves the poolTools names", () => { // ── Collection completeness ─────────────────────────────────────────────── -test("poolTools exposes exactly the five expected pool tools", () => { +test("poolTools exposes exactly the expected pool tools", () => { assert.deepEqual(Object.keys(poolTools).sort(), [...POOL_TOOL_NAMES].sort()); }); @@ -96,6 +102,7 @@ test("read tools require read:health; lifecycle tools require write:resilience", assert.deepEqual(tools.omniroute_pool_status.scopes, ["read:health"]); assert.deepEqual(tools.omniroute_pool_sessions.scopes, ["read:health"]); assert.deepEqual(tools.omniroute_pool_health.scopes, ["read:health"]); + assert.deepEqual(tools.omniroute_browser_pool_status.scopes, ["read:health"]); assert.deepEqual(tools.omniroute_pool_reset.scopes, ["write:resilience"]); assert.deepEqual(tools.omniroute_pool_warm.scopes, ["write:resilience"]); }); @@ -144,3 +151,47 @@ test("handlePoolReset reports reset:false for an unknown provider", async () => assert.equal(result.reset, false); assert.equal(result.provider, "no-such-provider-3368"); }); + +// ── #3368 PR7 — browser pool observability ──────────────────────────────── + +test("handleBrowserPoolStatus returns status + cumulative metrics shape", async () => { + const { __resetBrowserPoolMetricsForTest } = await import( + "../../open-sse/services/browserPool.ts" + ); + __resetBrowserPoolMetricsForTest(); + + const result = (await handleBrowserPoolStatus()) as { + status: { enabled: boolean; contexts: number; browserRunning: boolean }; + metrics: Record; + }; + + assert.equal(typeof result.status.enabled, "boolean"); + assert.equal(typeof result.status.contexts, "number"); + assert.equal(typeof result.status.browserRunning, "boolean"); + for (const key of [ + "browserLaunches", + "browserLaunchFailures", + "contextsCreated", + "contextsReused", + "contextsEvicted", + "contextsReleased", + "contextCreateFailures", + "shutdowns", + ]) { + assert.equal(typeof result.metrics[key], "number", `${key} must be a numeric counter`); + } + assert.equal(result.metrics.lastShutdownReason, null, "no shutdown yet → null reason"); +}); + +test("shutdownPool increments the shutdowns counter and records the reason", async () => { + const { shutdownPool, getBrowserPoolMetrics, __resetBrowserPoolMetricsForTest } = await import( + "../../open-sse/services/browserPool.ts" + ); + __resetBrowserPoolMetricsForTest(); + + await shutdownPool("unit-test-reason"); + + const { metrics } = getBrowserPoolMetrics(); + assert.equal(metrics.shutdowns, 1, "shutdownPool must increment the shutdowns counter"); + assert.equal(metrics.lastShutdownReason, "unit-test-reason"); +});