feat(settings): 9router-style Routing Strategy card + sticky parity#6678
feat(settings): 9router-style Routing Strategy card + sticky parity#6678SeaXen wants to merge 5 commits into
Conversation
… (ABI 148) (diegosouzapw#6605) fix(electron): bump electron 42→43 + rebuild better-sqlite3 from source against the Electron ABI (148). Electron 43 raises NODE_MODULE_VERSION to 148; better-sqlite3@12.11.1 has no electron-v148 prebuild, so the packaged app died with 'Nenhum driver SQLite disponível'. prepare-electron-standalone now compiles better-sqlite3 from source against the electron headers into build/Release (where 'bindings' resolves it). Validated by Electron Package Smoke (green) + local (node_register_module_v148). Supersedes diegosouzapw#6378. (--admin: the only reds are SonarQube/SonarCloud failing on a coverage-report artifact digest-mismatch — a GitHub Actions infra flake, not this diff; Sonar is green on main and the diff touches only the electron build.)
…iegosouzapw#6588) deps: bump the development group (6 updates). Rebased onto current main; all checks green after the electron-smoke fix (diegosouzapw#6605).
…7) + production deps bump (diegosouzapw#6620) fix(proxy): force CONNECT tunnel for HTTP proxied requests (undici 8.7) + production deps bump. undici 8.6+ changed ProxyAgent to forward plain-HTTP via request-proxy instead of CONNECT, breaking OAuth refresh through a connection proxy (501). proxyDispatcher now passes proxyTunnel:true. Validated: Unit Tests 3/8 (the OAuth-proxy test) green, new regression test green (fails without the fix on undici 8.7), SonarQube green. Supersedes diegosouzapw#6380. (--admin: the only red is Electron Package Smoke failing on a next-build artifact 'digest-mismatch' — a GitHub Actions infra flake corrupting the asar ('file data stream has unexpected number of bytes'); the better-sqlite3 rebuild itself succeeded (gyp ok) and the electron path is unchanged from diegosouzapw#6605 which passed the smoke. Not this diff.)
- Add RoutingStrategyCard on Settings → Routing (account RR, sticky, combo RR) - Add comboStrategy, comboStickyRoundRobinLimit, providerStrategies settings - Wire providerStrategies into multi-account auth selection - Resolve combo sticky via comboStickyRoundRobinLimit before account sticky - Per-provider account routing override on provider detail page
There was a problem hiding this comment.
Code Review
This pull request introduces a Routing Strategy configuration interface to achieve 9router parity, enabling global and per-provider overrides for account round-robin, sticky limits, and combo rotation. The reviewer feedback highlights several critical and high-severity issues that need to be addressed: a potential data-loss bug in the provider strategy saving logic where other provider strategies could be wiped out on failure, a missing import of clampStickyRoundRobinTargetLimit in rrState.ts, and UX issues in the input fields where clamping values on onChange prevents natural typing. Additionally, the feedback suggests catching errors in useEffect hooks and using optional chaining to prevent runtime errors.
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.
| const res = await fetch("/api/settings", { cache: "no-store" }); | ||
| const data = res.ok ? await res.json() : {}; |
There was a problem hiding this comment.
If the GET request to /api/settings fails or returns a non-OK response, res.ok is false and data defaults to {}. This means current (which holds all other provider strategies) will also be empty {}. When the subsequent PATCH request is sent, it will write { providerStrategies: { [providerKey]: override } }, which will completely wipe out all other provider strategies in the database.
To prevent this critical data loss, you should throw an error and abort the save operation if the GET request fails.
| const res = await fetch("/api/settings", { cache: "no-store" }); | |
| const data = res.ok ? await res.json() : {}; | |
| const res = await fetch("/api/settings", { cache: "no-store" }); | |
| if (!res.ok) throw new Error("Failed to fetch current settings"); | |
| const data = await res.json(); |
| if (perComboLimit !== undefined && perComboLimit !== null) { | ||
| return clampStickyRoundRobinTargetLimit(perComboLimit); | ||
| } | ||
| const comboSticky = settings?.comboStickyRoundRobinLimit; | ||
| if (comboSticky !== undefined && comboSticky !== null) { | ||
| return clampStickyRoundRobinTargetLimit(comboSticky); | ||
| } | ||
| return clampStickyRoundRobinTargetLimit(settings?.stickyRoundRobinLimit); |
There was a problem hiding this comment.
The function clampStickyRoundRobinTargetLimit is used in resolveComboStickyRoundRobinLimit but is not imported or defined in this file. This will cause a compilation or runtime error. Please ensure it is imported, or if it is defined in open-sse/services/combo.ts, move its definition to rrState.ts (or a shared utility file) to avoid a circular dependency between combo.ts and rrState.ts.
| if (nextStrategy === "round-robin" && nextSticky !== "") { | ||
| override.stickyRoundRobinLimit = Number(nextSticky) || 3; | ||
| } |
There was a problem hiding this comment.
The Zod schema in settingsSchemas.ts enforces stickyRoundRobinLimit: z.number().int().min(1).max(1000). If a user types or pastes an invalid value (e.g., 0 or a negative number), Number(nextSticky) || 3 could result in an invalid value being sent to the server, causing a Zod validation error (400 Bad Request) and failing to save. We should clamp the value on the client side before saving.
| if (nextStrategy === "round-robin" && nextSticky !== "") { | |
| override.stickyRoundRobinLimit = Number(nextSticky) || 3; | |
| } | |
| if (nextStrategy === "round-robin" && nextSticky !== "") { | |
| const val = parseInt(nextSticky, 10); | |
| override.stickyRoundRobinLimit = Math.min(10, Math.max(1, isNaN(val) ? 3 : val)); | |
| } |
| const data = await fetch("/api/settings", { cache: "no-store" }).then((r) => r.json()); | ||
| const override = ((data.providerStrategies || {}) as Record<string, ProviderStrategyOverride>)[ |
There was a problem hiding this comment.
Use optional chaining when accessing providerStrategies on data to prevent potential runtime TypeErrors if data is null, undefined, or not an object.
| const data = await fetch("/api/settings", { cache: "no-store" }).then((r) => r.json()); | |
| const override = ((data.providerStrategies || {}) as Record<string, ProviderStrategyOverride>)[ | |
| const data = await fetch("/api/settings", { cache: "no-store" }).then((r) => r.ok ? r.json() : null); | |
| const override = ((data?.providerStrategies || {}) as Record<string, ProviderStrategyOverride>)[ | |
| providerKey | |
| ]; |
| type RoutingSettings = { | ||
| fallbackStrategy?: string; | ||
| stickyRoundRobinLimit?: number; | ||
| comboStrategy?: string; | ||
| comboStickyRoundRobinLimit?: number; | ||
| }; |
There was a problem hiding this comment.
Update the RoutingSettings type definition to allow string values for the limit fields. This is necessary to support storing the raw string input in the state while the user is typing, before it is parsed and clamped on blur.
| type RoutingSettings = { | |
| fallbackStrategy?: string; | |
| stickyRoundRobinLimit?: number; | |
| comboStrategy?: string; | |
| comboStickyRoundRobinLimit?: number; | |
| }; | |
| type RoutingSettings = { | |
| fallbackStrategy?: string; | |
| stickyRoundRobinLimit?: number | string; | |
| comboStrategy?: string; | |
| comboStickyRoundRobinLimit?: number | string; | |
| }; |
| onChange={(e) => { | ||
| const num = Math.min(10, Math.max(1, parseInt(e.target.value, 10) || 3)); | ||
| setSettings((prev) => ({ ...prev, stickyRoundRobinLimit: num })); | ||
| }} | ||
| onBlur={() => | ||
| run(async () => { | ||
| const limit = settings.stickyRoundRobinLimit ?? 3; | ||
| const updated = await patchSettings({ stickyRoundRobinLimit: limit }); | ||
| setSettings((prev) => ({ | ||
| ...prev, | ||
| stickyRoundRobinLimit: updated.stickyRoundRobinLimit ?? limit, | ||
| })); | ||
| }) | ||
| } |
There was a problem hiding this comment.
Clamping and defaulting the value in the onChange handler makes it extremely difficult for users to type or edit the input. For example, if a user presses backspace to clear the input, parseInt("", 10) returns NaN, which immediately defaults to 3, preventing them from clearing the input.
Instead, allow the state to hold the raw string value during typing, and only clamp/validate on onBlur when saving.
| onChange={(e) => { | |
| const num = Math.min(10, Math.max(1, parseInt(e.target.value, 10) || 3)); | |
| setSettings((prev) => ({ ...prev, stickyRoundRobinLimit: num })); | |
| }} | |
| onBlur={() => | |
| run(async () => { | |
| const limit = settings.stickyRoundRobinLimit ?? 3; | |
| const updated = await patchSettings({ stickyRoundRobinLimit: limit }); | |
| setSettings((prev) => ({ | |
| ...prev, | |
| stickyRoundRobinLimit: updated.stickyRoundRobinLimit ?? limit, | |
| })); | |
| }) | |
| } | |
| onChange={(e) => { | |
| setSettings((prev) => ({ ...prev, stickyRoundRobinLimit: e.target.value })); | |
| }} | |
| onBlur={() => | |
| run(async () => { | |
| const limit = Math.min(10, Math.max(1, parseInt(String(settings.stickyRoundRobinLimit), 10) || 3)); | |
| const updated = await patchSettings({ stickyRoundRobinLimit: limit }); | |
| setSettings((prev) => ({ | |
| ...prev, | |
| stickyRoundRobinLimit: updated.stickyRoundRobinLimit ?? limit, | |
| })); | |
| }) | |
| } |
| onChange={(e) => { | ||
| const num = Math.min(100, Math.max(1, parseInt(e.target.value, 10) || 1)); | ||
| setSettings((prev) => ({ ...prev, comboStickyRoundRobinLimit: num })); | ||
| }} | ||
| onBlur={() => | ||
| run(async () => { | ||
| const limit = settings.comboStickyRoundRobinLimit ?? 1; | ||
| const updated = await patchSettings({ comboStickyRoundRobinLimit: limit }); | ||
| setSettings((prev) => ({ | ||
| ...prev, | ||
| comboStickyRoundRobinLimit: updated.comboStickyRoundRobinLimit ?? limit, | ||
| })); | ||
| }) | ||
| } |
There was a problem hiding this comment.
Similar to the stickyRoundRobinLimit input, clamping and defaulting the value in onChange prevents users from clearing the input or typing freely. Let's defer the clamping and validation to onBlur.
| onChange={(e) => { | |
| const num = Math.min(100, Math.max(1, parseInt(e.target.value, 10) || 1)); | |
| setSettings((prev) => ({ ...prev, comboStickyRoundRobinLimit: num })); | |
| }} | |
| onBlur={() => | |
| run(async () => { | |
| const limit = settings.comboStickyRoundRobinLimit ?? 1; | |
| const updated = await patchSettings({ comboStickyRoundRobinLimit: limit }); | |
| setSettings((prev) => ({ | |
| ...prev, | |
| comboStickyRoundRobinLimit: updated.comboStickyRoundRobinLimit ?? limit, | |
| })); | |
| }) | |
| } | |
| onChange={(e) => { | |
| setSettings((prev) => ({ ...prev, comboStickyRoundRobinLimit: e.target.value })); | |
| }} | |
| onBlur={() => | |
| run(async () => { | |
| const limit = Math.min(100, Math.max(1, parseInt(String(settings.comboStickyRoundRobinLimit), 10) || 1)); | |
| const updated = await patchSettings({ comboStickyRoundRobinLimit: limit }); | |
| setSettings((prev) => ({ | |
| ...prev, | |
| comboStickyRoundRobinLimit: updated.comboStickyRoundRobinLimit ?? limit, | |
| })); | |
| }) | |
| } |
| useEffect(() => { | ||
| load(); | ||
| }, [load]); |
There was a problem hiding this comment.
The load function is asynchronous and can throw errors (e.g., if the network request fails). Calling it inside useEffect without catching errors will result in an unhandled promise rejection.
| useEffect(() => { | |
| load(); | |
| }, [load]); | |
| useEffect(() => { | |
| load().catch(console.error); | |
| }, [load]); |
|
Addressed gemini-code-assist review in latest push:
Semgrep already passing on prior commit. |
Problem
Users comparing 9router Profile → Routing Strategy could not find equivalent controls in OmniRoute (sticky round-robin was buried under advanced combo defaults).
Solution
comboStrategy,comboStickyRoundRobinLimit,providerStrategiesproviderStrategieshonored inauth.ts; combo sticky usescomboStickyRoundRobinLimitbefore global account stickyHow to verify
Tests
tests/unit/combo-rr-sticky-9router.test.tssettings-ui-layout-static.test.tsupdated for card order