fix: isolate provider endpoint pools and sticky sessions#919
Merged
Conversation
📝 WalkthroughWalkthrough添加了多语言错误消息、增强的端点引用检查、新的批量会话终止 API、在 provider 编辑/删除流程中触发粘性会话失效，以及若干前端表单与测试调整以支持这些行为变更。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refines how provider endpoints and sessions are managed, focusing on improving isolation, user feedback, and system responsiveness. The changes ensure that provider configurations, especially for self-hosted instances, behave predictably and that any modifications to routing-sensitive fields correctly invalidate existing user sessions, prompting re-evaluation of routing decisions. This leads to a more robust and transparent provider management experience. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
Bot
added
size/L
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces several valuable fixes and improvements related to provider endpoints and sticky sessions. The changes to vendor clustering, UI forms for provider creation, and more detailed error messages for endpoint deletion are well-implemented. The addition of logic to terminate sticky sessions when routing configurations change is a crucial enhancement for system stability. The new regression tests provide good coverage for these changes. My only suggestion is to address a piece of duplicated code to improve maintainability.
Comment on lines
+205
to
+221
| async function terminateStickySessionsForProviders(providerIds: number[], context: string) { | ||
| const uniqueProviderIds = Array.from( | ||
| new Set(providerIds.filter((providerId) => Number.isInteger(providerId) && providerId > 0)) | ||
| ); | ||
| if (uniqueProviderIds.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await SessionManager.terminateProviderSessionsBatch(uniqueProviderIds); | ||
| } catch (error) { | ||
| logger.warn(`${context}:terminate_provider_sessions_failed`, { | ||
| providerIds: uniqueProviderIds, | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
This helper function terminateStickySessionsForProviders is also defined in src/actions/provider-endpoints.ts. To follow the DRY (Don't Repeat Yourself) principle, it's best to move this function to a shared location, like a new utility file under src/lib/utils/.
Furthermore, the try-catch block here is likely unreachable because SessionManager.terminateProviderSessionsBatch already handles its own errors and doesn't re-throw them. This makes the context parameter unused and the catch block dead code.
A good refactoring approach would be to have a single, shared helper function. If contextual logging from the actions is important, terminateProviderSessionsBatch could be modified to accept a context string for its error logs, which would make this wrapper unnecessary.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+205
to
+221
| async function terminateStickySessionsForProviders(providerIds: number[], context: string) { | ||
| const uniqueProviderIds = Array.from( | ||
| new Set(providerIds.filter((providerId) => Number.isInteger(providerId) && providerId > 0)) | ||
| ); | ||
| if (uniqueProviderIds.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await SessionManager.terminateProviderSessionsBatch(uniqueProviderIds); | ||
| } catch (error) { | ||
| logger.warn(`${context}:terminate_provider_sessions_failed`, { | ||
| providerIds: uniqueProviderIds, | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Duplicate helper across action files
terminateStickySessionsForProviders is duplicated identically in src/actions/provider-endpoints.ts (line 246) and here (line 205). Consider extracting it into a shared module (e.g., src/lib/session-manager.ts or a new src/lib/sticky-session-utils.ts) to avoid drift between the two copies.
This is non-blocking but will help maintainability if the error handling or filtering logic needs to change later.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/providers.ts
Line: 205-221
Comment:
**Duplicate helper across action files**
`terminateStickySessionsForProviders` is duplicated identically in `src/actions/provider-endpoints.ts` (line 246) and here (line 205). Consider extracting it into a shared module (e.g., `src/lib/session-manager.ts` or a new `src/lib/sticky-session-utils.ts`) to avoid drift between the two copies.
This is non-blocking but will help maintainability if the error handling or filtering logic needs to change later.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| @@ -13,7 +13,10 @@ const updateProviderEndpointMock = vi.fn(); | |||
| const findProviderEndpointProbeLogsBatchMock = vi.fn(); | |||
| const findVendorTypeEndpointStatsBatchMock = vi.fn(); | |||
| const hasEnabledProviderReferenceForVendorTypeUrlMock = vi.fn(); | |||
There was a problem hiding this comment.
Stale mock for removed import
hasEnabledProviderReferenceForVendorTypeUrlMock is still declared, registered in vi.mock, and has default values set in beforeEach, but the actual action code no longer imports hasEnabledProviderReferenceForVendorTypeUrl from @/repository/provider-endpoints. The new code uses findEnabledProviderReferencesForVendorTypeUrl instead.
While the stale mock won't cause test failures (the module mock still exports it), it creates confusion about which function the action code actually calls. Consider removing the mock declaration, its entry in vi.mock, and the beforeEach default at line 98 to keep the test in sync with the production code.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/actions/provider-endpoints.test.ts
Line: 15
Comment:
**Stale mock for removed import**
`hasEnabledProviderReferenceForVendorTypeUrlMock` is still declared, registered in `vi.mock`, and has default values set in `beforeEach`, but the actual action code no longer imports `hasEnabledProviderReferenceForVendorTypeUrl` from `@/repository/provider-endpoints`. The new code uses `findEnabledProviderReferencesForVendorTypeUrl` instead.
While the stale mock won't cause test failures (the module mock still exports it), it creates confusion about which function the action code actually calls. Consider removing the mock declaration, its entry in `vi.mock`, and the `beforeEach` default at line 98 to keep the test in sync with the production code.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/unit/repository/statistics-quota-costs-all-time.test.ts (1)
42-68: 重复代码：两个测试用例的 drizzle-orm mock 设置几乎完全相同这两个测试块中的 mock 设置代码（约 27 行）几乎完全重复。建议提取为共享的工厂函数，提高可维护性并确保修改时的一致性。
♻️ 建议的重构方案
+function createDrizzleMock(captureAndArgs: (args: unknown[]) => void) { + const sqlMock = Object.assign( + (strings: TemplateStringsArray, ...values: unknown[]) => ({ + queryChunks: [...strings, ...values], + }), + { + raw: vi.fn(), + join: vi.fn(), + identifier: vi.fn(), + } + ); + return { + relations: vi.fn(() => ({})), + eq: vi.fn((...args: unknown[]) => ({ kind: "eq", args })), + gte: vi.fn((...args: unknown[]) => ({ kind: "gte", args })), + inArray: vi.fn((...args: unknown[]) => ({ kind: "inArray", args })), + isNull: vi.fn((...args: unknown[]) => ({ kind: "isNull", args })), + lt: vi.fn((...args: unknown[]) => ({ kind: "lt", args })), + sql: sqlMock, + and: (...args: unknown[]) => { + captureAndArgs(args); + return { kind: "and", args }; + }, + }; +}然后在每个测试中使用：
vi.doMock("drizzle-orm", async () => { const actual = await vi.importActual<typeof import("drizzle-orm")>("drizzle-orm"); return { ...actual, ...createDrizzleMock((args) => { capturedAndArgs = args; }), }; });Also applies to: 106-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/repository/statistics-quota-costs-all-time.test.ts` around lines 42 - 68, Extract the repeated drizzle-orm mock into a shared factory function (e.g., createDrizzleMock) that returns the mocked members (relations, eq, gte, inArray, isNull, lt, sql: sqlMock, and) and accepts a callback to capture the `and` args so tests can set `capturedAndArgs`; then replace each `vi.doMock("drizzle-orm", async () => { ... })` block with a minimal wrapper that imports the actual module and spreads `...createDrizzleMock((args) => { capturedAndArgs = args })` into the returned object, reusing the `sqlMock`/`capturedAndArgs` behavior and eliminating the duplicated 27-line mock setup.src/actions/provider-endpoints.ts (1)
541-551: 会话终止范围可能过宽当编辑端点的
url时，当前实现使用findEnabledProviderIdsByVendorAndType获取该 vendor/type 下的所有已启用 provider，而非仅获取实际引用该端点 URL 的 provider。相比之下，deleteProviderEndpoint中的逻辑使用findEnabledProviderReferencesForVendorTypeUrl精确定位引用特定 URL 的 provider。建议在
url变更时采用更精确的查询，仅终止引用该 endpoint URL 的 provider 会话。对于sortOrder或isEnabled的独立变更，如果整个 vendor/type 端点池的调度逻辑会受影响，则可保持当前的广泛范围。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/provider-endpoints.ts` around lines 541 - 551, The current edit logic uses findEnabledProviderIdsByVendorAndType for any URL change and thus terminates sessions for all providers of the vendor/type; instead, when parsed.data.url is changed, call findEnabledProviderReferencesForVendorTypeUrl(endpoint.vendorId, endpoint.providerType, oldUrlOrEndpointUrl) to get only providers that actually reference that URL and pass those IDs to terminateStickySessionsForProviders("editProviderEndpoint"); keep using findEnabledProviderIdsByVendorAndType only for changes to sortOrder or isEnabled where broad termination is intended. Ensure you determine the old URL vs new URL as needed to query references precisely.tests/unit/settings/providers/provider-form-endpoint-pool.test.tsx (1)
14-18: 建议将src类型导入改为@/别名路径这里从
../../../../src/types/provider导入，和仓库路径规范不一致。建议直接使用@/types/provider，可读性和稳定性更好。建议修改
import enMessages from "../../../../messages/en"; import type { ProviderDisplay, ProviderEndpoint, ProviderVendor, -} from "../../../../src/types/provider"; +} from "@/types/provider";As per coding guidelines
**/*.{ts,tsx,js,jsx}: Use path alias@/to reference files in./src/directory.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/settings/providers/provider-form-endpoint-pool.test.tsx` around lines 14 - 18, The import of ProviderDisplay, ProviderEndpoint, and ProviderVendor is using a relative path "../../../../src/types/provider" which violates the repo alias convention; update the import in provider-form-endpoint-pool.test.tsx to use the path alias (import from "@/types/provider") so the test references the types via the project's '@/...' alias instead of the relative src path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/actions/providers.ts`:
- Around line 201-203: The current shouldInvalidateStickySessionsOnProviderEdit
function only checks presence of keys in the payload and thus invalidates sticky
sessions even when values are unchanged; change it to detect actual value
changes by comparing incoming data fields (filtered to
STICKY_SESSION_INVALIDATING_PROVIDER_FIELDS) against the existing provider's
current values (e.g., load the provider/entity by id used in the edit flow) and
return true only if at least one of those fields' values differ (use deep
equality for objects/arrays); update the call-sites (the edit handler that uses
shouldInvalidateStickySessionsOnProviderEdit, referenced around lines ~816) to
pass the existing provider state so the function can perform the comparison.
In `@src/lib/session-manager.ts`:
- Around line 2056-2072: The pipeline result handling in sessionIds collection
ignores per-command errors from pipeline.exec(), causing failed subcommands
(e.g., a zrange error) to be silently skipped; change the logic around
pipeline.exec() and the subsequent loop over results to explicitly detect and
log/handle errors for each entry (inspect the tuple items returned by
pipeline.exec(), identify error objects vs array results), e.g., check each
result for an Error or non-null error indicator before treating it as the zrange
response, surface or log the error with context (provider/id) and skip only when
safe, and ensure functions using sessionIds (the session deletion flow) account
for reported failures so deletions/counts remain accurate.
---
Nitpick comments:
In `@src/actions/provider-endpoints.ts`:
- Around line 541-551: The current edit logic uses
findEnabledProviderIdsByVendorAndType for any URL change and thus terminates
sessions for all providers of the vendor/type; instead, when parsed.data.url is
changed, call findEnabledProviderReferencesForVendorTypeUrl(endpoint.vendorId,
endpoint.providerType, oldUrlOrEndpointUrl) to get only providers that actually
reference that URL and pass those IDs to
terminateStickySessionsForProviders("editProviderEndpoint"); keep using
findEnabledProviderIdsByVendorAndType only for changes to sortOrder or isEnabled
where broad termination is intended. Ensure you determine the old URL vs new URL
as needed to query references precisely.
In `@tests/unit/repository/statistics-quota-costs-all-time.test.ts`:
- Around line 42-68: Extract the repeated drizzle-orm mock into a shared factory
function (e.g., createDrizzleMock) that returns the mocked members (relations,
eq, gte, inArray, isNull, lt, sql: sqlMock, and) and accepts a callback to
capture the `and` args so tests can set `capturedAndArgs`; then replace each
`vi.doMock("drizzle-orm", async () => { ... })` block with a minimal wrapper
that imports the actual module and spreads `...createDrizzleMock((args) => {
capturedAndArgs = args })` into the returned object, reusing the
`sqlMock`/`capturedAndArgs` behavior and eliminating the duplicated 27-line mock
setup.
In `@tests/unit/settings/providers/provider-form-endpoint-pool.test.tsx`:
- Around line 14-18: The import of ProviderDisplay, ProviderEndpoint, and
ProviderVendor is using a relative path "../../../../src/types/provider" which
violates the repo alias convention; update the import in
provider-form-endpoint-pool.test.tsx to use the path alias (import from
"@/types/provider") so the test references the types via the project's '@/...'
alias instead of the relative src path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f1264f87-e015-41a4-a0b8-db734b87d550
📒 Files selected for processing (20)
messages/en/errors.jsonmessages/ja/errors.jsonmessages/ru/errors.jsonmessages/zh-CN/errors.jsonmessages/zh-TW/errors.jsonsrc/actions/provider-endpoints.tssrc/actions/providers.tssrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/index.tsxsrc/app/[locale]/settings/providers/_components/provider-endpoints-table.tsxsrc/lib/provider-endpoint-error-codes.tssrc/lib/session-manager.tssrc/repository/provider-endpoints.tstests/unit/actions/provider-endpoints.test.tstests/unit/actions/providers.test.tstests/unit/lib/session-manager-terminate-provider-sessions.test.tstests/unit/repository/provider-endpoints-vendor-key.test.tstests/unit/repository/statistics-quota-costs-all-time.test.tstests/unit/settings/providers/provider-endpoints-table.test.tsxtests/unit/settings/providers/provider-form-endpoint-pool.test.tsx
Comment on lines
+201
to
+203
| function shouldInvalidateStickySessionsOnProviderEdit(data: Record<string, unknown>): boolean { | ||
| return Object.keys(data).some((key) => STICKY_SESSION_INVALIDATING_PROVIDER_FIELDS.has(key)); | ||
| } |
There was a problem hiding this comment.
会话失效条件按“字段存在”判断会误杀未变更会话
Line 202 当前只要 payload 里出现目标字段就会触发失效。即使值未变化（例如编辑表单原样提交），Line 816 仍会终止 sticky sessions，和“仅在路由面发生变化时失效”的目标不一致。
建议修改（基于“实际变更字段”触发）
-const STICKY_SESSION_INVALIDATING_PROVIDER_FIELDS = new Set([
- "url",
- "website_url",
- "provider_type",
- "group_tag",
- "is_enabled",
- "allowed_models",
- "allowed_clients",
- "blocked_clients",
- "model_redirects",
- "active_time_start",
- "active_time_end",
-]);
+const STICKY_SESSION_INVALIDATING_PROVIDER_FIELDS = new Set<keyof Provider>([
+ "url",
+ "websiteUrl",
+ "providerType",
+ "groupTag",
+ "isEnabled",
+ "allowedModels",
+ "allowedClients",
+ "blockedClients",
+ "modelRedirects",
+ "activeTimeStart",
+ "activeTimeEnd",
+]);
-function shouldInvalidateStickySessionsOnProviderEdit(data: Record<string, unknown>): boolean {
- return Object.keys(data).some((key) => STICKY_SESSION_INVALIDATING_PROVIDER_FIELDS.has(key));
+function shouldInvalidateStickySessionsOnProviderEdit(changedFields: Set<keyof Provider>): boolean {
+ return [...changedFields].some((key) => STICKY_SESSION_INVALIDATING_PROVIDER_FIELDS.has(key));
}+ const changedProviderFields = new Set<keyof Provider>();
const preimageFields: Record<string, unknown> = {};
for (const [field, nextValue] of Object.entries(payload)) {
if (field === "key") {
continue;
}
@@
if (!hasProviderFieldChangedForUndo(currentValue, nextValue)) {
continue;
}
preimageFields[providerKey] = currentValue;
+ changedProviderFields.add(providerKey);
}
@@
- if (shouldInvalidateStickySessionsOnProviderEdit(payload)) {
+ if (shouldInvalidateStickySessionsOnProviderEdit(changedProviderFields)) {
await terminateStickySessionsForProviders([providerId], "editProvider");
}Also applies to: 816-818
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/providers.ts` around lines 201 - 203, The current
shouldInvalidateStickySessionsOnProviderEdit function only checks presence of
keys in the payload and thus invalidates sticky sessions even when values are
unchanged; change it to detect actual value changes by comparing incoming data
fields (filtered to STICKY_SESSION_INVALIDATING_PROVIDER_FIELDS) against the
existing provider's current values (e.g., load the provider/entity by id used in
the edit flow) and return true only if at least one of those fields' values
differ (use deep equality for objects/arrays); update the call-sites (the edit
handler that uses shouldInvalidateStickySessionsOnProviderEdit, referenced
around lines ~816) to pass the existing provider state so the function can
perform the comparison.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
|
|
||
| const sessionIds = new Set<string>(); | ||
| for (const [, result] of results) { |
Contributor
There was a problem hiding this comment.
[HIGH] [ERROR-SILENT] terminateProviderSessionsBatch ignores per-command Redis pipeline errors
Evidence (src/lib/session-manager.ts:2062):
for (const [, result] of results) {
if (!Array.isArray(result)) {
continue;
}pipeline.exec() returns [err, result] tuples. Dropping err means individual zrange failures (wrong key type, moved slot, transient Redis error) will be silently skipped and session termination can become a no-op without any log.
Suggested fix:
for (const [index, [err, result]] of results.entries()) {
const providerId = uniqueProviderIds[index];
if (err) {
logger.warn("SessionManager: Failed to read provider active sessions", {
providerId,
error: err,
});
continue;
}
if (!Array.isArray(result)) continue;
for (const sessionId of result) {
if (typeof sessionId === "string" && sessionId.trim()) {
sessionIds.add(sessionId);
}
}
}
| "active_time_end", | ||
| ]); | ||
|
|
||
| function shouldInvalidateStickySessionsOnProviderEdit(data: Record<string, unknown>): boolean { |
Contributor
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Sticky-session invalidation checks only for key presence, not value changes
Evidence (src/actions/providers.ts:201):
function shouldInvalidateStickySessionsOnProviderEdit(data: Record<string, unknown>): boolean {
return Object.keys(data).some((key) => STICKY_SESSION_INVALIDATING_PROVIDER_FIELDS.has(key));
}This triggers termination whenever the caller includes any of these fields, even if the value is unchanged (e.g., callers that submit a full provider payload on edit).
Suggested fix (compare against currentProvider before calling updateProvider):
const STICKY_SESSION_INVALIDATING_FIELD_TO_PROVIDER_KEY: Record<string, keyof Provider> = {
url: "url",
website_url: "websiteUrl",
provider_type: "providerType",
group_tag: "groupTag",
is_enabled: "isEnabled",
allowed_models: "allowedModels",
allowed_clients: "allowedClients",
blocked_clients: "blockedClients",
model_redirects: "modelRedirects",
active_time_start: "activeTimeStart",
active_time_end: "activeTimeEnd",
};
function shouldInvalidateStickySessionsOnProviderEdit(
next: Record<string, unknown>,
current: Provider
): boolean {
return Object.entries(next).some(([field, value]) => {
const providerKey = STICKY_SESSION_INVALIDATING_FIELD_TO_PROVIDER_KEY[field];
if (!providerKey) return false;
return hasProviderFieldChangedForUndo(current[providerKey], value);
});
}
// ...inside editProvider
if (shouldInvalidateStickySessionsOnProviderEdit(payload, currentProvider)) {
await terminateStickySessionsForProviders([providerId], "editProvider");
}
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR is size L and spans provider routing/session invalidation, endpoint pooling/vendor key logic, UI error surfacing, i18n message updates, and several test fixtures. The main risks are (1) sticky sessions being terminated more broadly than intended and (2) Redis pipeline per-command failures being ignored during termination.
PR Size: L
- Lines changed: 824
- Files changed: 20
- Split suggestions:
- PR 1:
SessionManager.terminateProviderSessionsBatch+ action hooks + unit tests - PR 2: vendor key/endpoint pooling changes in
src/repository/provider-endpoints.ts+ repo tests - PR 3: UI delete error translation +
messages/*/errors.json+ UI tests - PR 4: unrelated test fixture/mocking adjustments (e.g.,
statistics-quota-costs-all-time.test.ts)
- PR 1:
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 1 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
src/actions/providers.ts:201[HIGH] [LOGIC-BUG] Sticky-session invalidation checks only key presence (Confidence: 85/100)src/lib/session-manager.ts:2062[HIGH] [ERROR-SILENT] Redis pipeline per-command errors ignored in provider session termination (Confidence: 85/100)
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
Contributor
There was a problem hiding this comment.
- PR detected:
#919(fix: isolate provider endpoint pools and sticky sessions) - Applied size label:
size/L(824 lines, 20 files) - Posted 2 inline review comments:
src/actions/providers.ts:201— sticky-session invalidation triggers on key presence, not actual value changes (can terminate sessions on “no-op” edits)src/lib/session-manager.ts:2062—terminateProviderSessionsBatchignores per-command Redis pipeline errors (can silently skip termination)
- Submitted the required review summary via
gh pr review --comment
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR implements provider endpoint pool isolation and sticky session management with comprehensive test coverage. Overall, the changes are well-structured and address the linked issue #918 effectively.
PR Size: L
- Lines changed: ~600 (after excluding test fixture churn)
- Files changed: 18
- Note: This is a well-scoped PR that touches related areas (endpoint management, session handling, and i18n). No split suggested.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 1 |
| Simplification | 0 | 0 | 0 | 2 |
Low Priority Issues (Author's Discretion)
1. Duplicate helper function across action files
- File:
src/actions/providers.ts(~line 226) andsrc/actions/provider-endpoints.ts(~line 113) - Issue: The
terminateStickySessionsForProvidershelper is duplicated identically in both files - Suggestion: Extract to a shared module (e.g.,
src/lib/session-manager.tsor a new shared utility) to avoid maintenance drift
2. Stale mock in test file
- File:
tests/unit/actions/provider-endpoints.test.ts(lines 15, 67, 87, 347) - Issue:
hasEnabledProviderReferenceForVendorTypeUrlMockis still declared and configured, but the action code now usesfindEnabledProviderReferencesForVendorTypeUrlinstead - Suggestion: Remove the stale mock declarations and cleanup references
Review Coverage
- Logic and correctness - Clean implementation of sticky session termination
- Security (OWASP Top 10) - No security issues identified
- Error handling - Proper try/catch with logging, graceful fallbacks
- Type safety - Proper TypeScript types maintained
- Documentation accuracy - Comments explain the intent clearly
- Test coverage - Comprehensive tests added for new behaviors
- Code clarity - Good separation of concerns, clear naming
Highlights
- Vendor key port preservation: The fix to
normalizeWebsiteDomainKeyFromUrlcorrectly preserves explicit ports (e.g.,example.com:3000) for self-hosted provider clustering - Provider form isolation: Clean removal of vendor resolution in create/clone modes prevents unintended endpoint pool attachment
- Actionable delete errors: The new
ENDPOINT_REFERENCED_BY_ENABLED_PROVIDERSerror code with provider name summary improves UX significantly - Session termination: Best-effort session termination with proper error logging and fallbacks
Recommendation
Approve with minor suggestions. The code duplication and stale mock are non-blocking but worth addressing for long-term maintainability.
Automated review by Claude AI
4 tasks
- Extract duplicate terminateStickySessionsForProviders into SessionManager static method (DRY) - Fix sticky session invalidation to check actual value changes via preimageFields instead of raw payload keys (logic bug) - Add missing allowed_clients/blocked_clients to preimage mapping - Handle Redis pipeline errors in terminateProviderSessionsBatch instead of silently dropping them - Remove stale hasEnabledProviderReferenceForVendorTypeUrlMock from tests
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
Comment on lines
+629
to
+636
| const affectedProviderIds = await findEnabledProviderIdsByVendorAndType( | ||
| endpoint.vendorId, | ||
| endpoint.providerType | ||
| ); | ||
| await SessionManager.terminateStickySessionsForProviders( | ||
| affectedProviderIds, | ||
| "removeProviderEndpoint" | ||
| ); |
There was a problem hiding this comment.
Unguarded DB call after soft-delete can produce misleading error
findEnabledProviderIdsByVendorAndType is called outside any try-catch at line 629, after the endpoint has already been soft-deleted at line 610. Every other non-critical post-delete operation in this function (resetEndpointCircuitState, tryDeleteProviderVendorIfEmpty, publishProviderCacheInvalidation) is wrapped in its own try-catch precisely to prevent this scenario. If the DB query at line 629 throws (e.g. transient DB connectivity issue), the outer catch at line 660 returns { ok: false, errorCode: DELETE_FAILED } — even though the deletion already succeeded. The UI and the caller will believe the operation failed and likely show an error, while the endpoint is actually gone.
Suggested change
| const affectedProviderIds = await findEnabledProviderIdsByVendorAndType( | |
| endpoint.vendorId, | |
| endpoint.providerType | |
| ); | |
| await SessionManager.terminateStickySessionsForProviders( | |
| affectedProviderIds, | |
| "removeProviderEndpoint" | |
| ); | |
| try { | |
| const affectedProviderIds = await findEnabledProviderIdsByVendorAndType( | |
| endpoint.vendorId, | |
| endpoint.providerType | |
| ); | |
| await SessionManager.terminateStickySessionsForProviders( | |
| affectedProviderIds, | |
| "removeProviderEndpoint" | |
| ); | |
| } catch (error) { | |
| logger.warn("removeProviderEndpoint:terminate_sessions_failed", { | |
| endpointId: parsed.data.endpointId, | |
| vendorId: endpoint.vendorId, | |
| error: error instanceof Error ? error.message : String(error), | |
| }); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/provider-endpoints.ts
Line: 629-636
Comment:
**Unguarded DB call after soft-delete can produce misleading error**
`findEnabledProviderIdsByVendorAndType` is called outside any try-catch at line 629, after the endpoint has already been soft-deleted at line 610. Every other non-critical post-delete operation in this function (`resetEndpointCircuitState`, `tryDeleteProviderVendorIfEmpty`, `publishProviderCacheInvalidation`) is wrapped in its own try-catch precisely to prevent this scenario. If the DB query at line 629 throws (e.g. transient DB connectivity issue), the outer catch at line 660 returns `{ ok: false, errorCode: DELETE_FAILED }` — even though the deletion already succeeded. The UI and the caller will believe the operation failed and likely show an error, while the endpoint is actually gone.
```suggestion
try {
const affectedProviderIds = await findEnabledProviderIdsByVendorAndType(
endpoint.vendorId,
endpoint.providerType
);
await SessionManager.terminateStickySessionsForProviders(
affectedProviderIds,
"removeProviderEndpoint"
);
} catch (error) {
logger.warn("removeProviderEndpoint:terminate_sessions_failed", {
endpointId: parsed.data.endpointId,
vendorId: endpoint.vendorId,
error: error instanceof Error ? error.message : String(error),
});
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+2076
to
2121
| } | ||
| } | ||
| } | ||
|
|
||
| if (sessionIds.size === 0) { | ||
| return 0; | ||
| } | ||
|
|
||
| const terminatedCount = await SessionManager.terminateSessionsBatch([...sessionIds]); | ||
| logger.info("SessionManager: Terminated provider sessions batch", { | ||
| providerIds: uniqueProviderIds, | ||
| sessionCount: sessionIds.size, | ||
| terminatedCount, | ||
| }); | ||
| return terminatedCount; | ||
| } catch (error) { | ||
| logger.error("SessionManager: Failed to terminate provider sessions batch", { | ||
| error, | ||
| providerIds: uniqueProviderIds, | ||
| }); | ||
| return 0; | ||
| } | ||
| } | ||
|
|
||
| static async terminateStickySessionsForProviders( | ||
| providerIds: number[], | ||
| context: string | ||
| ): Promise<void> { | ||
| const uniqueProviderIds = Array.from( | ||
| new Set(providerIds.filter((providerId) => Number.isInteger(providerId) && providerId > 0)) | ||
| ); | ||
| if (uniqueProviderIds.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await SessionManager.terminateProviderSessionsBatch(uniqueProviderIds); | ||
| } catch (error) { | ||
| logger.warn(`${context}:terminate_provider_sessions_failed`, { | ||
| providerIds: uniqueProviderIds, | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Dead catch block in terminateStickySessionsForProviders
terminateProviderSessionsBatch internally catches all errors (via its own try-catch at the bottom of the method) and always returns 0 on failure — it never propagates exceptions. As a result, the catch block in terminateStickySessionsForProviders can never be triggered, making it unreachable dead code. This could mislead future maintainers into thinking the wrapper provides meaningful error isolation that it does not.
If the goal is to provide a fire-and-forget, non-throwing wrapper, consider documenting that terminateProviderSessionsBatch is already non-throwing, and simplify:
static async terminateStickySessionsForProviders(
providerIds: number[],
context: string
): Promise<void> {
const uniqueProviderIds = Array.from(
new Set(providerIds.filter((id) => Number.isInteger(id) && id > 0))
);
if (uniqueProviderIds.length === 0) return;
// terminateProviderSessionsBatch is non-throwing; it catches all errors internally
await SessionManager.terminateProviderSessionsBatch(uniqueProviderIds);
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/session-manager.ts
Line: 2076-2121
Comment:
**Dead `catch` block in `terminateStickySessionsForProviders`**
`terminateProviderSessionsBatch` internally catches all errors (via its own try-catch at the bottom of the method) and always returns `0` on failure — it never propagates exceptions. As a result, the `catch` block in `terminateStickySessionsForProviders` can never be triggered, making it unreachable dead code. This could mislead future maintainers into thinking the wrapper provides meaningful error isolation that it does not.
If the goal is to provide a fire-and-forget, non-throwing wrapper, consider documenting that `terminateProviderSessionsBatch` is already non-throwing, and simplify:
```ts
static async terminateStickySessionsForProviders(
providerIds: number[],
context: string
): Promise<void> {
const uniqueProviderIds = Array.from(
new Set(providerIds.filter((id) => Number.isInteger(id) && id > 0))
);
if (uniqueProviderIds.length === 0) return;
// terminateProviderSessionsBatch is non-throwing; it catches all errors internally
await SessionManager.terminateProviderSessionsBatch(uniqueProviderIds);
}
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/actions/provider-endpoints.ts (1)
523-536:⚠️ Potential issue | 🟠 Major只在值真的变化时再终止 sticky sessions。
这里还是按字段“是否出现在 payload 里”判断。管理员如果原样提交
url/sortOrder/isEnabled，即使路由面没有变化，也会把同 vendor/type 下的会话全部清掉，和前面 provider 编辑路径里已经修过的问题是同一类。可直接改成和 `previous` 的实际值比较
- const shouldTerminateStickySessions = - parsed.data.url !== undefined || - parsed.data.sortOrder !== undefined || - parsed.data.isEnabled !== undefined; + const shouldTerminateStickySessions = + (parsed.data.url !== undefined && parsed.data.url !== previous.url) || + (parsed.data.sortOrder !== undefined && + parsed.data.sortOrder !== previous.sortOrder) || + (parsed.data.isEnabled !== undefined && parsed.data.isEnabled !== previous.isEnabled);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/provider-endpoints.ts` around lines 523 - 536, The current check uses presence in parsed.data to decide termination, which wipes sessions even if values are unchanged; update the shouldTerminateStickySessions logic to compare parsed.data fields to the existing endpoint values instead (e.g. check parsed.data.url !== undefined && parsed.data.url !== endpoint.url, parsed.data.sortOrder !== undefined && parsed.data.sortOrder !== endpoint.sortOrder, parsed.data.isEnabled !== undefined && parsed.data.isEnabled !== endpoint.isEnabled) and only call findEnabledProviderIdsByVendorAndType and SessionManager.terminateStickySessionsForProviders when at least one of those real-value comparisons indicates a change.
🧹 Nitpick comments (1)
tests/unit/actions/providers.test.ts (1)
579-595: 把allowed_clients/blocked_clients也单独锁进回归测试。这条用例目前只会在
group_tag或allowed_models回归时失败；而本次源码真正新增的 preimage 映射是allowed_clients/blocked_clients。如果这两个 key 以后再次漏掉，这里不会报警。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/actions/providers.test.ts` around lines 579 - 595, The test only asserts termination when group_tag or allowed_models change, so extend this test (or add a new one) to also exercise edits to allowed_clients and blocked_clients: call editProvider(1, { allowed_clients: [...], blocked_clients: [...] }) (or include them in the existing payload), assert updateProviderMock was called with objectContaining these keys (allowed_clients, blocked_clients) and assert terminateProviderSessionsBatchMock was called with [1], "editProvider"; reference the existing editProvider, updateProviderMock and terminateProviderSessionsBatchMock symbols when adding the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/actions/provider-endpoints.ts`:
- Around line 227-244: The function formatProviderReferenceSummary is assembling
user-facing summary text (e.g., "A, B, C +1") which bypasses i18n; instead,
change formatProviderReferenceSummary (and the similar logic around the other
occurrence) to stop returning formatted strings and return raw data for the
presentation layer — e.g., return an object with unique provider names array and
a total count (or just the names array and length) so the UI can compose
localized text using errorCode/errorParams; remove any hardcoded concatenation
or "+N" formatting from the action layer.
---
Duplicate comments:
In `@src/actions/provider-endpoints.ts`:
- Around line 523-536: The current check uses presence in parsed.data to decide
termination, which wipes sessions even if values are unchanged; update the
shouldTerminateStickySessions logic to compare parsed.data fields to the
existing endpoint values instead (e.g. check parsed.data.url !== undefined &&
parsed.data.url !== endpoint.url, parsed.data.sortOrder !== undefined &&
parsed.data.sortOrder !== endpoint.sortOrder, parsed.data.isEnabled !==
undefined && parsed.data.isEnabled !== endpoint.isEnabled) and only call
findEnabledProviderIdsByVendorAndType and
SessionManager.terminateStickySessionsForProviders when at least one of those
real-value comparisons indicates a change.
---
Nitpick comments:
In `@tests/unit/actions/providers.test.ts`:
- Around line 579-595: The test only asserts termination when group_tag or
allowed_models change, so extend this test (or add a new one) to also exercise
edits to allowed_clients and blocked_clients: call editProvider(1, {
allowed_clients: [...], blocked_clients: [...] }) (or include them in the
existing payload), assert updateProviderMock was called with objectContaining
these keys (allowed_clients, blocked_clients) and assert
terminateProviderSessionsBatchMock was called with [1], "editProvider";
reference the existing editProvider, updateProviderMock and
terminateProviderSessionsBatchMock symbols when adding the assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b8467ae-d0de-4b39-9815-c06891065c8b
📒 Files selected for processing (5)
src/actions/provider-endpoints.tssrc/actions/providers.tssrc/lib/session-manager.tstests/unit/actions/provider-endpoints.test.tstests/unit/actions/providers.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/session-manager.ts
Comment on lines
+227
to
+244
| function formatProviderReferenceSummary( | ||
| references: Array<{ id: number; name: string }>, | ||
| maxDisplayCount: number = 3 | ||
| ): string { | ||
| const uniqueNames = Array.from( | ||
| new Set(references.map((reference) => reference.name.trim()).filter(Boolean)) | ||
| ); | ||
| if (uniqueNames.length === 0) { | ||
| return ""; | ||
| } | ||
|
|
||
| if (uniqueNames.length <= maxDisplayCount) { | ||
| return uniqueNames.join(", "); | ||
| } | ||
|
|
||
| const displayed = uniqueNames.slice(0, maxDisplayCount).join(", "); | ||
| return `${displayed} +${uniqueNames.length - maxDisplayCount}`; | ||
| } |
There was a problem hiding this comment.
不要在 action 层拼装新的用户可见文案。
这里新增的 error 中文句子，以及 providers 的 "A, B, C +1" 摘要，都会绕过现有 i18n 流程。既然已经有 errorCode 和 errorParams，更稳妥的是只返回原始数据（例如 provider names/count），由展示层按 locale 组装文案。
As per coding guidelines, "All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text"
Also applies to: 593-606
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/provider-endpoints.ts` around lines 227 - 244, The function
formatProviderReferenceSummary is assembling user-facing summary text (e.g., "A,
B, C +1") which bypasses i18n; instead, change formatProviderReferenceSummary
(and the similar logic around the other occurrence) to stop returning formatted
strings and return raw data for the presentation layer — e.g., return an object
with unique provider names array and a total count (or just the names array and
length) so the UI can compose localized text using errorCode/errorParams; remove
any hardcoded concatenation or "+N" formatting from the action layer.
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes critical provider endpoint isolation bugs where vendor clustering incorrectly merged self-hosted providers, create/clone forms auto-attached to shared endpoint pools, endpoint deletion provided no actionable errors, and sticky sessions persisted after routing configuration changes.
Problem
Issue #918 - Provider endpoint logic had multiple interrelated defects:
Port-ignoring vendor clustering: URLs like
192.168.1.1:111and192.168.1.1:222were treated as the same vendor because only hostname was used for clusteringClone/create inheritance: Cloning a provider automatically inherited the original's endpoint pool, making it impossible to create truly independent providers
Silent URL replacement: The create/clone form would silently replace user-provided URLs with endpoints from shared pools
Undiagnosable delete failures: Endpoint deletion failures gave no information about which providers were blocking the operation
Stale sticky sessions: After endpoint/provider changes, existing sessions continued routing to old configurations until manual termination or TTL expiration
Related Issues:
Solution
1. Port-Aware Vendor Clustering
Modified
normalizeWebsiteDomainKeyFromUrlto preserve explicit ports:https://example.com→ key =example.com(no port)https://example.com:3000→ key =example.com:3000(port preserved)2. Create/Clone Form Isolation
Refactored provider form:
websiteUrlin create/clone modes3. Actionable Delete Errors
Enhanced endpoint deletion:
ENDPOINT_REFERENCED_BY_ENABLED_PROVIDERSwith i18n4. Sticky Session Termination
Added session invalidation:
SessionManager.terminateProviderSessionsBatchmethodChanges
Core Changes
src/repository/provider-endpoints.tssrc/lib/session-manager.tsterminateProviderSessionsBatchmethodsrc/actions/providers.tssrc/actions/provider-endpoints.tssrc/app/[locale]/settings/providers/_components/forms/provider-form/index.tsxsrc/app/[locale]/settings/providers/_components/provider-endpoints-table.tsxSupporting Changes
Testing
Automated Tests
terminateProviderSessionsBatchVerification Commands
Checklist
Original Greptile Analysis
Greptile Summary
This PR fixes five interrelated provider endpoint isolation bugs: port-ignoring vendor clustering, create/clone forms silently inheriting shared endpoint pools, uninformative delete-conflict errors, and stale sticky sessions persisting after routing configuration changes. The fixes are well-scoped and each change is backed by new unit tests.
Key changes:
normalizeWebsiteDomainKeyFromUrlnow preserves explicit ports in the vendor clustering key, so192.168.1.1:111and192.168.1.1:222are no longer merged into the same vendorwebsiteUrlor auto-fill the URL from a shared endpoint pool — users must provide explicit endpoint configurationremoveProviderEndpointnow returns an i18n-translated error (ENDPOINT_REFERENCED_BY_ENABLED_PROVIDERS) with the count and names of blocking providers instead of a generic conflictSessionManager.terminateProviderSessionsBatchadded; called after endpoint edits/deletes and provider edits/removes to evict stale sticky sessions from Redisallowed_clients/blocked_clientswere missing fromSINGLE_EDIT_PREIMAGE_FIELD_TO_PROVIDER_KEY, meaning changes to those fields via the single-edit path never triggered session invalidation — this is now fixedIssues found:
removeProviderEndpoint, the newfindEnabledProviderIdsByVendorAndTypeDB call (line 629) sits outside any try-catch after the soft-delete has already committed. A transient DB error here would cause the outer catch to returnDELETE_FAILEDeven though the deletion succeeded — unlike the other post-delete calls (resetEndpointCircuitState,tryDeleteProviderVendorIfEmpty,publishProviderCacheInvalidation) which are all individually guardedcatchblock interminateStickySessionsForProvidersis unreachable becauseterminateProviderSessionsBatchalready swallows all its own exceptions internallyerrorfallback string for theENDPOINT_REFERENCED_BY_ENABLED_PROVIDERSerror path is still a hardcoded Chinese string, inconsistent with the i18n approach used elsewhereConfidence Score: 3/5
findEnabledProviderIdsByVendorAndTypecall after the soft-deleteImportant Files Changed
normalizeWebsiteDomainKeyFromUrl; two new queriesfindEnabledProviderReferencesForVendorTypeUrlandfindEnabledProviderIdsByVendorAndTypeadded. Logic is sound and well-tested.terminateProviderSessionsBatchandterminateStickySessionsForProvidersmethods added. Logic is correct butterminateStickySessionsForProviderscontains an unreachablecatchblock sinceterminateProviderSessionsBatchalready swallows all exceptions.editProviderEndpointandremoveProviderEndpoint. InremoveProviderEndpoint, the newfindEnabledProviderIdsByVendorAndTypeDB call is unguarded after the soft-delete, which can surface a misleading DELETE_FAILED response if the query throws while the deletion already succeeded. Also retains a hardcoded Chineseerrorfallback string.editProviderandremoveProvider; routing-affecting field set defined. Also addsallowed_clients/blocked_clientsto the preimage field map, fixing a missing mapping that would have prevented session invalidation for those edits.errorCode/errorParamsrather than a generic toast. TheObject.assignpattern for attachingactionResultto the thrown Error is clean and type-safe.Sequence Diagram
sequenceDiagram participant UI as UI (provider-endpoints-table) participant AE as actions/provider-endpoints participant AR as repository/provider-endpoints participant SM as SessionManager participant Redis as Redis Note over UI,Redis: removeProviderEndpoint flow UI->>AE: removeProviderEndpoint({ endpointId }) AE->>AR: findEnabledProviderReferencesForVendorTypeUrl(vendorId, type, url) AR-->>AE: references[] alt references.length > 0 AE-->>UI: { ok: false, errorCode: ENDPOINT_REFERENCED_BY_ENABLED_PROVIDERS, errorParams } UI->>UI: toast.error(getErrorMessage(tErrors, errorCode, errorParams)) else no references AE->>AR: softDeleteProviderEndpoint(endpointId) AE->>AR: findEnabledProviderIdsByVendorAndType(vendorId, type) AR-->>AE: providerIds[] AE->>SM: terminateStickySessionsForProviders(providerIds) SM->>SM: terminateProviderSessionsBatch(providerIds) SM->>Redis: pipeline.zrange(provider:N:active_sessions) Redis-->>SM: sessionIds[] SM->>SM: terminateSessionsBatch(sessionIds) AE-->>UI: { ok: true } end Note over UI,Redis: editProviderEndpoint flow (url/sortOrder/isEnabled changed) UI->>AE: editProviderEndpoint({ endpointId, isEnabled: false }) AE->>AR: updateProviderEndpoint(endpointId, data) AR-->>AE: updated endpoint AE->>AR: findEnabledProviderIdsByVendorAndType(vendorId, type) AR-->>AE: providerIds[] AE->>SM: terminateStickySessionsForProviders(providerIds) SM->>Redis: pipeline + terminateSessionsBatch AE-->>UI: { ok: true, data: { endpoint } }Prompt To Fix All With AI
Last reviewed commit: 22c68ac