feat: 新增代理池并拆分请求超时配置#131
Merged
Merged
Conversation
新增更多-代理池，支持维护 http/https/socks5 代理，并将系统设置和标准分组上游代理改为从代理池选择；升级迁移会一次性清空旧代理配置，后续重建代理池后不再重复清空。 代理池支持手动测试、检测全部和默认 1 小时的页面内定期检测，单个代理显示 10 秒测试超时，检测错误会脱敏代理凭据；代理新增/编辑弹框改为与现有表单弹框一致的卡片式布局。 拆分非流式/流式请求超时配置，并调整标准分组与聚合分组超时后的重试行为；补充代理池保存、分组代理保存、代理流转、迁移幂等性、超时重试和代理检测相关测试。
|
Warning Review limit reached
More reviews will be available in 5 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a proxy-pool feature (DB model, migrations, service, handlers, routes, frontend UI, and API client), introduces proxy URL parsing/normalization, splits request timeouts into non-stream/stream semantics applied across runtime paths (HTTP, SSE, retries), updates dynamic-weight health/weight floor, and extends tests and locales. ChangesProxy-pool and timeout flow
Possibly related PRs
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/utils/proxy_url.go (1)
16-17: 💤 Low valueConsider more specific error messages for proxy URL validation failures.
The current error message "invalid proxy URL" doesn't distinguish between a URL parse failure and missing scheme/host, which could make debugging harder for users configuring proxies.
💡 Optional refinement for clearer diagnostics
parsed, err := url.Parse(trimmed) - if err != nil || parsed.Scheme == "" || parsed.Host == "" { - return "", fmt.Errorf("invalid proxy URL") + if err != nil { + return "", fmt.Errorf("invalid proxy URL: %w", err) + } + if parsed.Scheme == "" || parsed.Host == "" { + return "", fmt.Errorf("invalid proxy URL: missing scheme or host") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/utils/proxy_url.go` around lines 16 - 17, The error returned from proxy URL validation is too generic; update the validation in the function that parses the proxy URL (the code using parsed, parsed.Scheme, parsed.Host and err) to return more specific errors: when err != nil return an error that wraps or includes the parse error, and when parsed.Scheme == "" or parsed.Host == "" return distinct errors like "proxy URL missing scheme" or "proxy URL missing host" (include the invalid input for context). Ensure you reference and preserve the original parse error when available so callers can see the underlying failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/src/features/proxy-pool/components/ProxyPoolPanel.vue`:
- Around line 254-271: The testAll function currently runs testItem sequentially
causing long blocks; update testAll to run tests concurrently in controlled
batches: keep the existing guards and flags (batchTesting, testingAll, items),
but instead of for-awaiting each item call, partition items.value into chunks
(e.g., size 5) and for each chunk call Promise.all on chunk.map(i => testItem(i,
true)) and await that before proceeding to the next batch; ensure errors from
testItem are caught per-item or propagated so the finally block still clears
testingAll and batchTesting and keep the existing success message behavior when
!silent.
---
Nitpick comments:
In `@internal/utils/proxy_url.go`:
- Around line 16-17: The error returned from proxy URL validation is too
generic; update the validation in the function that parses the proxy URL (the
code using parsed, parsed.Scheme, parsed.Host and err) to return more specific
errors: when err != nil return an error that wraps or includes the parse error,
and when parsed.Scheme == "" or parsed.Host == "" return distinct errors like
"proxy URL missing scheme" or "proxy URL missing host" (include the invalid
input for context). Ensure you reference and preserve the original parse error
when available so callers can see the underlying failure.
🪄 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: 3f2c1fa3-f980-4150-af56-fce6e9462151
📒 Files selected for processing (38)
internal/app/app.gointernal/channel/factory.gointernal/channel/factory_test.gointernal/config/system_settings.gointernal/config/system_settings_test.gointernal/container/container.gointernal/db/migrations/migration.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.gointernal/handler/handler.gointernal/handler/proxy_pool_handler.gointernal/httpclient/manager.gointernal/i18n/locales/en-US.gointernal/i18n/locales/ja-JP.gointernal/i18n/locales/zh-CN.gointernal/models/types.gointernal/proxy/cc_support.gointernal/proxy/cc_support_test.gointernal/proxy/server.gointernal/proxy/server_test.gointernal/router/router.gointernal/services/group_service.gointernal/services/group_service_test.gointernal/services/proxy_pool_service.gointernal/services/proxy_pool_service_test.gointernal/types/types.gointernal/utils/config_utils.gointernal/utils/proxy_url.goweb/src/api/proxy-pool.tsweb/src/components/NavBar.vueweb/src/components/keys/GroupFormModal.vueweb/src/features/proxy-pool/components/ProxyPoolPanel.vueweb/src/locales/en-US.tsweb/src/locales/ja-JP.tsweb/src/locales/zh-CN.tsweb/src/types/models.tsweb/src/views/More.vueweb/src/views/Settings.vue
代理池“测试全部”改为固定小批次并发执行，避免大量代理按 10 秒超时逐个串行检测导致等待时间过长，同时保留批量检测状态与完成提示逻辑。 代理 URL 校验补充分类型错误信息，并新增回归测试覆盖裸 host:port、缺少 host 和解析失败时不泄漏代理凭据的场景。
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
internal/config/system_settings_test.go (1)
433-456: ⚡ Quick winAdd one effective-config test for non-zero stream timeout.
Current new tests only assert
stream_request_timeout = 0. Please add a case withstream_request_timeout > 0to lock the split-timeout behavior and prevent regressions in runtime timeout selection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/config/system_settings_test.go` around lines 433 - 456, Add a new unit test (similar to TestGetEffectiveConfigSplitTimeouts) that exercises SystemSettingsManager.GetEffectiveConfig with a non-zero "stream_request_timeout" value (e.g., map[string]any{"non_stream_request_timeout": float64(45), "stream_request_timeout": float64(30)}), then assert that cfg.StreamRequestTimeout equals the provided non-zero value, cfg.NonStreamRequestTimeout equals the provided non-stream value, and cfg.RequestTimeout equals the chosen runtime timeout behavior (ensure RequestTimeout reflects the correct selection when stream timeout > 0). Target the TestGetEffectiveConfigSplitTimeouts area and reference SystemSettingsManager.GetEffectiveConfig and the cfg fields NonStreamRequestTimeout, StreamRequestTimeout, RequestTimeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/db/migrations/v1_26_0_ClearLegacyProxyURLs.go`:
- Around line 29-35: The current migration checks
hasDataMigrationRun(clearLegacyProxyURLsMigrationVersion) outside the
transaction, allowing two instances to race and one to fail on duplicate marker
insert; modify the migration so the marker insert is idempotent under concurrent
startups by either (a) performing the check+insert inside the same transaction
or (b) catching a duplicate-key/unique-constraint error on the marker insertion
and treating it as success; update the code paths that call hasDataMigrationRun
and the function that writes the marker (referencing hasDataMigrationRun and
clearLegacyProxyURLsMigrationVersion and the marker-insert code in this file and
the similar block around lines 57-67) to implement one of these strategies so
duplicate-marker insertion does not cause startup errors.
In `@internal/proxy/server_test.go`:
- Around line 697-703: The test's "timeout retry" setup uses slowUpstream which
sleeps 150ms then returns 503, so with non_stream_request_timeout=1 it exercises
503 failover instead of the timeout path; update the slowUpstream handler (and
the other similar handlers referenced in the diff for the same test) to simulate
a real timeout by delaying longer than the proxy's request timeout and not
writing a response (e.g., block/sleep past the configured timeout and avoid
calling w.WriteHeader), ensuring the proxy's timeout logic (and aggregate
split-timeout retry behavior) is triggered instead of a 503 response.
- Around line 625-630: The test is missing an explicit precondition that the
request context is actually cancelled before calling
ps.shouldAbortOnIgnorableError; add an assertion right after <-ctx.Done() that
the request context error is set (e.g. assert.NotNil(t,
c.Request.Context().Err()) or assert.Equal(t, context.DeadlineExceeded,
c.Request.Context().Err())) so the test truly exercises the ignorable-error
branch when invoking ps.shouldAbortOnIgnorableError(c, err).
In `@internal/services/proxy_pool_service_test.go`:
- Line 124: The test currently does an unbounded receive from the proxyRequests
channel which can block forever; replace the direct receive (<-proxyRequests)
with a timed receive using a select (or testing helper) to fail the test if no
message arrives within a short timeout: wait on proxyRequests and on time.After,
then on success assert.Equal(t, targetURL, received) and on timeout call
t.Fatalf or assert.Failf to produce a deterministic test failure; reference the
proxyRequests channel and the targetURL variable in the select-based receive
logic inside this test.
In `@web/src/features/proxy-pool/components/ProxyPoolPanel.vue`:
- Around line 246-249: The catch block for the test request currently only shows
an error message and does not update testResults[item.id], which leaves stale
success data visible after a failed retest; inside the catch for the
function/method that performs the test (where testResults[item.id] is set on
success), set testResults[item.id] to an object representing failure (e.g.,
status: "failed" or success: false and any relevant error info) and keep the
existing message.error(t("proxyPool.testRequestFailed")) behavior so UI reflects
the failed state.
---
Nitpick comments:
In `@internal/config/system_settings_test.go`:
- Around line 433-456: Add a new unit test (similar to
TestGetEffectiveConfigSplitTimeouts) that exercises
SystemSettingsManager.GetEffectiveConfig with a non-zero
"stream_request_timeout" value (e.g.,
map[string]any{"non_stream_request_timeout": float64(45),
"stream_request_timeout": float64(30)}), then assert that
cfg.StreamRequestTimeout equals the provided non-zero value,
cfg.NonStreamRequestTimeout equals the provided non-stream value, and
cfg.RequestTimeout equals the chosen runtime timeout behavior (ensure
RequestTimeout reflects the correct selection when stream timeout > 0). Target
the TestGetEffectiveConfigSplitTimeouts area and reference
SystemSettingsManager.GetEffectiveConfig and the cfg fields
NonStreamRequestTimeout, StreamRequestTimeout, RequestTimeout.
🪄 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: 6dd8693e-9559-4b4b-b789-dd204ed4804f
📒 Files selected for processing (39)
internal/app/app.gointernal/channel/factory.gointernal/channel/factory_test.gointernal/config/system_settings.gointernal/config/system_settings_test.gointernal/container/container.gointernal/db/migrations/migration.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.gointernal/handler/handler.gointernal/handler/proxy_pool_handler.gointernal/httpclient/manager.gointernal/i18n/locales/en-US.gointernal/i18n/locales/ja-JP.gointernal/i18n/locales/zh-CN.gointernal/models/types.gointernal/proxy/cc_support.gointernal/proxy/cc_support_test.gointernal/proxy/server.gointernal/proxy/server_test.gointernal/router/router.gointernal/services/group_service.gointernal/services/group_service_test.gointernal/services/proxy_pool_service.gointernal/services/proxy_pool_service_test.gointernal/types/types.gointernal/utils/config_utils.gointernal/utils/proxy_url.gointernal/utils/proxy_url_test.goweb/src/api/proxy-pool.tsweb/src/components/NavBar.vueweb/src/components/keys/GroupFormModal.vueweb/src/features/proxy-pool/components/ProxyPoolPanel.vueweb/src/locales/en-US.tsweb/src/locales/ja-JP.tsweb/src/locales/zh-CN.tsweb/src/types/models.tsweb/src/views/More.vueweb/src/views/Settings.vue
让旧代理清理迁移在事务内抢占幂等标记，避免多实例启动时重复执行迁移或因重复标记写入失败。 补强代理池与超时相关测试：覆盖非零流式超时配置、聚合非流式真实超时重试、代理检测请求等待超时保护，并在前端检测请求异常时刷新失败状态，避免旧成功结果残留。
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/db/migrations/v1_26_0_ClearLegacyProxyURLs.go (1)
27-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
data_migrationstable creation can still race on first multi-instance rollout.
ensureDataMigrationsTablestill does a TOCTOUHasTable+CreateTablecheck/act (lines 76-80) before the migration’s transactional marker acquisition runs (lines 27-29). On an empty database, two nodes can concurrently attemptCREATE TABLE data_migrations, causing one node to fail beforeacquireClearLegacyProxyURLsMigrationMarkercan safely no-op viaON CONFLICT DO NOTHING.Suggested fix
Make the DDL concurrency-safe instead of relying on
HasTable/CreateTable(and avoidAutoMigrateas a “safe” alternative): use dialect-nativeCREATE TABLE IF NOT EXISTSfordata_migrations, or unconditionally attempt creation and ignore the “table already exists” error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/db/migrations/v1_26_0_ClearLegacyProxyURLs.go` around lines 27 - 29, ensureDataMigrationsTable currently does a TOCTOU HasTable + CreateTable which can race when multiple instances start; instead make the DDL creation idempotent and race-safe so acquireClearLegacyProxyURLsMigrationMarker can safely run. Replace the HasTable/CreateTable flow in ensureDataMigrationsTable with a dialect-native "CREATE TABLE IF NOT EXISTS" (or attempt creation and explicitly ignore "table already exists" errors) for the data_migrations table, ensuring the function returns any other errors; keep acquireClearLegacyProxyURLsMigrationMarker logic unchanged so its ON CONFLICT DO NOTHING stays the transaction-safe marker.internal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.go (1)
19-21:⚠️ Potential issue | 🟠 MajorFix flaky migration tests:
:memory:with GORM can use multiple SQLite connectionsSQLite
:memory:is per-connection, and GORM uses a connection pool—so migrations done on one connection may not be visible to later queries on another (leading to intermittentno such table/ missing rows). Apply the pool cap fix in both DB setup blocks (aroundinternal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.go:19-21and72-75).Suggested fix
db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) require.NoError(t, err) +sqlDB, err := db.DB() +require.NoError(t, err) +sqlDB.SetMaxOpenConns(1) require.NoError(t, db.AutoMigrate(&models.SystemSetting{}, &models.Group{}))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.go` around lines 19 - 21, Tests use gorm.Open(sqlite.Open(":memory:")) which is connection-local and GORM uses a pool, causing flaky missing-table errors; after opening the DB (the gorm.Open(...) call where you assign db, err and call db.AutoMigrate(...)) grab the underlying *sql.DB via sqlDB, _ := db.DB() and call sqlDB.SetMaxOpenConns(1) to force a single connection, and apply this same change to both DB setup blocks in the file (the blocks around the gorm.Open/sqlite.Open(":memory:") + AutoMigrate calls).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.go`:
- Around line 19-21: Tests use gorm.Open(sqlite.Open(":memory:")) which is
connection-local and GORM uses a pool, causing flaky missing-table errors; after
opening the DB (the gorm.Open(...) call where you assign db, err and call
db.AutoMigrate(...)) grab the underlying *sql.DB via sqlDB, _ := db.DB() and
call sqlDB.SetMaxOpenConns(1) to force a single connection, and apply this same
change to both DB setup blocks in the file (the blocks around the
gorm.Open/sqlite.Open(":memory:") + AutoMigrate calls).
In `@internal/db/migrations/v1_26_0_ClearLegacyProxyURLs.go`:
- Around line 27-29: ensureDataMigrationsTable currently does a TOCTOU HasTable
+ CreateTable which can race when multiple instances start; instead make the DDL
creation idempotent and race-safe so acquireClearLegacyProxyURLsMigrationMarker
can safely run. Replace the HasTable/CreateTable flow in
ensureDataMigrationsTable with a dialect-native "CREATE TABLE IF NOT EXISTS" (or
attempt creation and explicitly ignore "table already exists" errors) for the
data_migrations table, ensuring the function returns any other errors; keep
acquireClearLegacyProxyURLsMigrationMarker logic unchanged so its ON CONFLICT DO
NOTHING stays the transaction-safe marker.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0101f66f-2142-4e1f-96b0-f345a83533d7
📒 Files selected for processing (6)
internal/config/system_settings_test.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.gointernal/proxy/server_test.gointernal/services/proxy_pool_service_test.goweb/src/features/proxy-pool/components/ProxyPoolPanel.vue
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/proxy/server_test.go
- web/src/features/proxy-pool/components/ProxyPoolPanel.vue
- internal/services/proxy_pool_service_test.go
将 data_migrations 建表改为 CREATE TABLE IF NOT EXISTS，避免多实例首次启动时 HasTable/CreateTable 竞态导致迁移失败。 迁移测试统一限制 SQLite 内存库为单连接，避免 GORM 连接池切换连接时出现表或数据不可见的偶发失败。
Owner
Author
|
@coderabbitai full review |
1 similar comment
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
✅ Action performedFull review finished. |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
internal/proxy/cc_support_test.go (1)
4984-4984: ⚡ Quick winAvoid coupling test behavior to case name strings.
Line 4984 uses
tt.name == "zero config values uses preset"to decide setup, which is fragile if the case title is edited. Add an explicit boolean (e.g.,withGroup) in the table instead.Suggested refactor
tests := []struct { name string responseHeaderTimeout int streamRequestTimeout int + withGroup bool expectedFirstByteTimeout time.Duration expectedSubsequentTimeout time.Duration }{ { name: "no group in context uses preset values", responseHeaderTimeout: 0, streamRequestTimeout: 0, + withGroup: false, expectedFirstByteTimeout: sseFirstByteTimeoutPreset, expectedSubsequentTimeout: sseSubsequentTimeoutPreset, }, @@ { name: "zero config values uses preset", responseHeaderTimeout: 0, streamRequestTimeout: 0, + withGroup: true, expectedFirstByteTimeout: sseFirstByteTimeoutPreset, expectedSubsequentTimeout: sseSubsequentTimeoutPreset, }, } @@ - if tt.responseHeaderTimeout > 0 || tt.streamRequestTimeout > 0 || tt.name == "zero config values uses preset" { + if tt.withGroup {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/proxy/cc_support_test.go` at line 4984, The test currently branches on the case title string (tt.name == "zero config values uses preset") which is fragile; add an explicit boolean field (e.g., withGroup bool) to the test case struct in the table and use that instead of comparing tt.name. Update the test cases to set withGroup for the relevant case(s), change the conditional at the setup site to check tt.withGroup (or the chosen field name) alongside the existing tt.responseHeaderTimeout/tt.streamRequestTimeout checks, and remove the string-based comparison to make the behavior robust to renaming.web/src/components/keys/GroupFormModal.vue (1)
311-313: ⚡ Quick winProxy options can go stale after the first successful load
Line 311 gates refresh behind
proxyPoolFetched, so newly added/edited proxies won’t appear in later modal opens during the same session.Suggested change
- if (!proxyPoolFetched.value) { - fetchProxyPoolOptions(); - } + fetchProxyPoolOptions();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/keys/GroupFormModal.vue` around lines 311 - 313, The proxy options are only fetched once because proxyPoolFetched gates calls to fetchProxyPoolOptions; to ensure newly added/edited proxies appear on subsequent modal opens, change the logic in GroupFormModal.vue so the modal triggers a fresh fetch instead of relying solely on proxyPoolFetched — either call fetchProxyPoolOptions unconditionally when the modal opens/when show/edit mode is set, or reset proxyPoolFetched to false on modal open before calling fetchProxyPoolOptions; update the code paths that open the modal to use these functions (reference proxyPoolFetched and fetchProxyPoolOptions) so options are reloaded each time.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/handler/proxy_pool_handler.go`:
- Around line 40-42: The handler currently wraps every error from
ProxyPoolService.Create/Update/Test as ErrValidation; instead, detect if the
returned err is already an API error (use errors.As / type assertion to
*app_errors.APIError) or comes from DB parsing (app_errors.ParseDBError) and
forward it unchanged to response.Error, otherwise wrap unknown errors as
ErrInternal (or a more appropriate class) with err.Error(); update the error
handling in the Create, Update, and Test handling blocks in
proxy_pool_handler.go to preserve and re-use the original app_errors error type
instead of always using app_errors.ErrValidation.
In `@internal/proxy/cc_support.go`:
- Around line 4200-4203: The SSE idle-timeout branch currently treats
cfg.StreamRequestTimeout == 0 as "not smaller than preset" and thus keeps the
60s preset, violating the semantics that 0 disables stream timeout; update the
logic in the stream timeout selection so that when cfg.StreamRequestTimeout <= 0
you do not apply the preset timeout (i.e., treat 0 as “no timeout”), and
likewise update SSEReaderWithTimeout.ReadEvent to call readEventInternal() when
timeout <= 0 so it runs in no-timeout mode; refer to cfg.StreamRequestTimeout,
subsequentTimeout, and SSEReaderWithTimeout.ReadEvent/readEventInternal to
locate the changes.
In `@internal/proxy/server_test.go`:
- Around line 670-675: The test is accidentally exercising the http.Client
timeout instead of the proxy's NonStreamRequestTimeout; update the test so the
client does not have a short Timeout (remove or set http.Client{Timeout} to 0 or
a value > NonStreamRequestTimeout) and make the upstream delay reflect the
non-stream timeout path (e.g. delay longer than NonStreamRequestTimeout rather
than 150ms) so the code paths exercising NonStreamRequestTimeout (refer to
upstream handler, attempts atomic counter, and NonStreamRequestTimeout) are
actually validated.
In `@web/src/features/proxy-pool/components/ProxyPoolPanel.vue`:
- Around line 158-176: The icon-only NButton instances that call openEdit(row)
and confirmDelete(row) (using CreateOutline and TrashOutline) lack accessible
names; add descriptive accessible labels (e.g., aria-label="Edit proxy" and
aria-label="Delete proxy") to those NButton props so screen readers can announce
the action—use localized strings if i18n is available (e.g., $t('edit') /
$t('delete')) and keep the existing onClick handlers (openEdit and
confirmDelete) intact.
- Around line 64-70: The validator in ProxyPoolPanel.vue currently uses a regex
that only checks a prefix so malformed trailing input can pass; update the
validator (the validator: (_rule, value: string) => { ... } function) to trim
the value and perform a full-string match by anchoring the regex (add $) and
disallowing whitespace (e.g., use \S+ or [^\s]+) so the entire input must be a
valid proxy URL (protocol://host[...] ) — replace the existing pattern with one
that ends with $ and test against the trimmed value.
---
Nitpick comments:
In `@internal/proxy/cc_support_test.go`:
- Line 4984: The test currently branches on the case title string (tt.name ==
"zero config values uses preset") which is fragile; add an explicit boolean
field (e.g., withGroup bool) to the test case struct in the table and use that
instead of comparing tt.name. Update the test cases to set withGroup for the
relevant case(s), change the conditional at the setup site to check tt.withGroup
(or the chosen field name) alongside the existing
tt.responseHeaderTimeout/tt.streamRequestTimeout checks, and remove the
string-based comparison to make the behavior robust to renaming.
In `@web/src/components/keys/GroupFormModal.vue`:
- Around line 311-313: The proxy options are only fetched once because
proxyPoolFetched gates calls to fetchProxyPoolOptions; to ensure newly
added/edited proxies appear on subsequent modal opens, change the logic in
GroupFormModal.vue so the modal triggers a fresh fetch instead of relying solely
on proxyPoolFetched — either call fetchProxyPoolOptions unconditionally when the
modal opens/when show/edit mode is set, or reset proxyPoolFetched to false on
modal open before calling fetchProxyPoolOptions; update the code paths that open
the modal to use these functions (reference proxyPoolFetched and
fetchProxyPoolOptions) so options are reloaded each time.
🪄 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: 238810f0-b529-4d78-b4ac-5d32547a8dff
📒 Files selected for processing (39)
internal/app/app.gointernal/channel/factory.gointernal/channel/factory_test.gointernal/config/system_settings.gointernal/config/system_settings_test.gointernal/container/container.gointernal/db/migrations/migration.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.gointernal/handler/handler.gointernal/handler/proxy_pool_handler.gointernal/httpclient/manager.gointernal/i18n/locales/en-US.gointernal/i18n/locales/ja-JP.gointernal/i18n/locales/zh-CN.gointernal/models/types.gointernal/proxy/cc_support.gointernal/proxy/cc_support_test.gointernal/proxy/server.gointernal/proxy/server_test.gointernal/router/router.gointernal/services/group_service.gointernal/services/group_service_test.gointernal/services/proxy_pool_service.gointernal/services/proxy_pool_service_test.gointernal/types/types.gointernal/utils/config_utils.gointernal/utils/proxy_url.gointernal/utils/proxy_url_test.goweb/src/api/proxy-pool.tsweb/src/components/NavBar.vueweb/src/components/keys/GroupFormModal.vueweb/src/features/proxy-pool/components/ProxyPoolPanel.vueweb/src/locales/en-US.tsweb/src/locales/ja-JP.tsweb/src/locales/zh-CN.tsweb/src/types/models.tsweb/src/views/More.vueweb/src/views/Settings.vue
- 保留代理池服务返回的 API 错误类型，避免数据库或未找到错误被误报为参数校验，并补充错误类型回归测试。 - 修正 Claude/Codex SSE 空闲超时对 stream_request_timeout=0 的处理，补齐无后续超时测试，并让非流式重试测试真正验证代理自身超时。 - 分组弹窗每次打开刷新代理池选项，收紧代理 URL 前端校验，并为代理池编辑、删除图标按钮补充可访问标签。
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/app/app.go (1)
184-206:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix migration sequencing: v1_27_0 runs before its table exists.
Line 184 calls
V1_27_0_AddProxyPoolNameUniqueIndexbeforeAutoMigrate, but theproxy_pool_itemstable is created byAutoMigrateat line 196. The v1_27_0 migration (which deduplicates names and adds a unique index) cannot operate on a non-existent table. Additionally, line 204'sMigrateDatabaseinvokes v1_27_0 again, causing duplicate execution.Remove the direct call at line 184. The migration should only run inside
MigrateDatabase(line 204) after the table exists.🐛 Proposed fix
// Database migration dbmigrations.HandleLegacyIndexes(a.db) - if err := dbmigrations.V1_27_0_AddProxyPoolNameUniqueIndex(a.db); err != nil { - return fmt.Errorf("database proxy pool name migration failed: %w", err) - } if err := a.db.AutoMigrate(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/app/app.go` around lines 184 - 206, Remove the premature direct call to dbmigrations.V1_27_0_AddProxyPoolNameUniqueIndex so the v1_27_0 migration does not run before AutoMigrate creates the proxy_pool_items table and to avoid duplicate execution (it will be run by dbmigrations.MigrateDatabase after AutoMigrate). Concretely, delete the block that invokes V1_27_0_AddProxyPoolNameUniqueIndex in the init/startup sequence (the call immediately before a.db.AutoMigrate) and keep the AutoMigrate call and the later dbmigrations.MigrateDatabase invocation unchanged so the v1_27_0 migration runs only once after the tables exist.
🧹 Nitpick comments (1)
internal/proxy/server_test.go (1)
833-837: ⚡ Quick winUse condition-based synchronization instead of fixed sleep in aggregate timeout retry test.
The
time.Sleep(200 * time.Millisecond)gate is timing-sensitive and can produce flaky behavior on slow CI runners. Prefer signaling based on first slow-attempt entry to make the test deterministic.Proposed refactor
- go func() { - time.Sleep(200 * time.Millisecond) - // Test scaffolding: keep fastGroup unavailable for the first selection, then restore fastKey outside upstream handlers. - _ = memStore.LPush(activeKeysListKeyForTest(uint64(fastGroup.ID)), uint64(fastKey.ID)) - }() + firstSlowAttemptStarted := make(chan struct{}) + slowUpstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + atomic.AddInt32(&slowAttempts, 1) + select { + case <-firstSlowAttemptStarted: + default: + close(firstSlowAttemptStarted) + } + time.Sleep(1200 * time.Millisecond) + })) + // ... + go func() { + <-firstSlowAttemptStarted + _ = memStore.LPush(activeKeysListKeyForTest(uint64(fastGroup.ID)), uint64(fastKey.ID)) + }()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/proxy/server_test.go` around lines 833 - 837, Replace the fixed time.Sleep in the goroutine that pushes fastKey into memStore (currently using memStore.LPush and activeKeysListKeyForTest with fastGroup/fastKey) with condition-based synchronization: have the test signal the goroutine (e.g., via a channel or sync.Cond) when the first slow-attempt entry occurs, and only then perform memStore.LPush to restore the fastKey; locate the gate logic around the anonymous go func and the upstream slow-attempt hook, add a channel (or waitgroup/cond) that the slow-attempt path closes/sends to when entered, and make the goroutine wait on that signal instead of sleeping so the retry test becomes deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/services/group_service.go`:
- Around line 2789-2793: The backfill unconditionally copies
configMap["request_timeout"] to configMap["non_stream_request_timeout"], which
can overwrite an explicit new-key zero and remove intended fallback behavior;
change the logic to only backfill when the legacy value is present and non-zero
(and non-empty) — e.g., check legacyValue != "0" (or parse to int and ensure >0)
before assigning to configMap["non_stream_request_timeout"]; apply the same
guarded check to the other identical block that also touches these keys.
In `@web/src/components/keys/GroupFormModal.vue`:
- Around line 674-684: The catch path in fetchProxyPoolOptions leaves
proxyPoolOptions.value unchanged so stale entries can persist; update the error
handling in fetchProxyPoolOptions (the block that calls proxyPoolApi.list) to
clear proxyPoolOptions.value (set to an empty array) when an error occurs and
still call message.error(t("proxyPool.loadFailed")) so the modal won't show
deleted proxies after a failed reload.
In `@web/src/features/proxy-pool/components/ProxyPoolPanel.vue`:
- Around line 371-393: loadProxyPoolSettings currently swallows fetch failures
and the caller opens the modal before settings finish loading, allowing a Save
to overwrite real server values; change loadProxyPoolSettings to surface
success/failure (e.g. return true on success, false or throw on error) and make
the modal-opening code await the promise (or check the boolean) before showing
the modal. Specifically, keep applyProxyPoolSettings(...) inside
loadProxyPoolSettings on successful fetch, but return a success indicator (or
rethrow the caught error) from loadProxyPoolSettings, and update the modal
opener (e.g. openProxyPoolModal / the method that triggers showing the
ProxyPoolPanel) to await loadProxyPoolSettings() and only display the modal when
the call succeeded.
- Around line 241-258: The page-count computation should clamp zero-record
results to show at least one page: update the totalPages computed (and any
dependent logic) so when total.value === 0 it returns 1 (keep the existing
negative/unknown behavior when total.value < 0), e.g. compute
Math.ceil(total.value / pageSize.value) for positive totals but enforce a
minimum of 1; leave totalRecordsText and pageInfoText as-is so they will use the
clamped totalPages and display "Page 1 of 1" instead of "Page 1 of 0" (refer to
the totalPages, totalRecordsText, pageInfoText, total, pageSize, and currentPage
symbols).
---
Outside diff comments:
In `@internal/app/app.go`:
- Around line 184-206: Remove the premature direct call to
dbmigrations.V1_27_0_AddProxyPoolNameUniqueIndex so the v1_27_0 migration does
not run before AutoMigrate creates the proxy_pool_items table and to avoid
duplicate execution (it will be run by dbmigrations.MigrateDatabase after
AutoMigrate). Concretely, delete the block that invokes
V1_27_0_AddProxyPoolNameUniqueIndex in the init/startup sequence (the call
immediately before a.db.AutoMigrate) and keep the AutoMigrate call and the later
dbmigrations.MigrateDatabase invocation unchanged so the v1_27_0 migration runs
only once after the tables exist.
---
Nitpick comments:
In `@internal/proxy/server_test.go`:
- Around line 833-837: Replace the fixed time.Sleep in the goroutine that pushes
fastKey into memStore (currently using memStore.LPush and
activeKeysListKeyForTest with fastGroup/fastKey) with condition-based
synchronization: have the test signal the goroutine (e.g., via a channel or
sync.Cond) when the first slow-attempt entry occurs, and only then perform
memStore.LPush to restore the fastKey; locate the gate logic around the
anonymous go func and the upstream slow-attempt hook, add a channel (or
waitgroup/cond) that the slow-attempt path closes/sends to when entered, and
make the goroutine wait on that signal instead of sleeping so the retry test
becomes deterministic.
🪄 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: aa76ff44-56b2-4b46-ab2c-eb8a6c815b08
📒 Files selected for processing (44)
internal/app/app.gointernal/channel/factory.gointernal/channel/factory_test.gointernal/config/system_settings.gointernal/config/system_settings_test.gointernal/container/container.gointernal/db/migrations/migration.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.gointernal/db/migrations/v1_27_0_AddProxyPoolNameUniqueIndex.gointernal/db/migrations/v1_27_0_AddProxyPoolNameUniqueIndex_test.gointernal/handler/handler.gointernal/handler/proxy_pool_handler.gointernal/handler/proxy_pool_handler_test.gointernal/handler/test_main_test.gointernal/httpclient/manager.gointernal/i18n/locales/en-US.gointernal/i18n/locales/ja-JP.gointernal/i18n/locales/locales_test.gointernal/i18n/locales/zh-CN.gointernal/models/types.gointernal/proxy/cc_support.gointernal/proxy/cc_support_test.gointernal/proxy/server.gointernal/proxy/server_test.gointernal/router/router.gointernal/services/group_service.gointernal/services/group_service_test.gointernal/services/proxy_pool_service.gointernal/services/proxy_pool_service_test.gointernal/types/types.gointernal/utils/config_utils.gointernal/utils/proxy_url.gointernal/utils/proxy_url_test.goweb/src/api/proxy-pool.tsweb/src/components/NavBar.vueweb/src/components/keys/GroupFormModal.vueweb/src/features/proxy-pool/components/ProxyPoolPanel.vueweb/src/locales/en-US.tsweb/src/locales/ja-JP.tsweb/src/locales/zh-CN.tsweb/src/types/models.tsweb/src/views/More.vueweb/src/views/Settings.vue
- 移除启动阶段 AutoMigrate 前的代理池名称唯一索引迁移直调，改由 AutoMigrate 后的统一数据迁移入口执行，并补充回归测试防止顺序回退。 - 收紧 legacy request_timeout 兼容逻辑：仅正数旧值回填 split timeout，兼容可解析的旧字符串数值，同时保留显式 non_stream_request_timeout=0 的禁用语义并补测试。 - 优化聚合超时重试测试同步，避免固定 sleep；缺少 stream 字段的普通请求按非流式超时处理，并补充回归测试。 - 调整聚合分组连续硬失败健康度衰减曲线，2 到 10 次连续失败采用不同惩罚，最低健康度仍保留可恢复的最小有效权重；补充聚合重试跨渠道时非最终失败 attempt 也更新健康度的测试。 - 代理池选项加载失败清空旧数据，空分页显示为 1 页，检测设置加载成功后再打开弹窗。
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
调整动态权重健康度计算：连续硬失败按阶梯曲线更快降权，0% 成功率在样本达到阈值后按请求数限制健康度上限，连续 8 次与 10 次失败显示不同；启用目标的有效权重最低保留 1.0，避免彻底失去恢复机会。 更新相关回归测试与展示说明，并将聚合分组新增子分组弹窗的默认权重从 10 改为 100。
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/services/dynamic_weight.go (1)
653-663: ⚡ Quick winSimplify the critical-health branch to an explicit
1.0recovery weight.This clamp sequence now always resolves to
1.0for enabled weights, so keepingbaseWeight * 0.1here is dead logic and keeps outdated “10%” semantics in the code path.Suggested simplification
- // Critical health: use minimum 10% of base weight to allow recovery - // Cap at 1.0 to prevent unhealthy high-weight targets from dominating healthy low-weight targets - // Example: baseWeight=100 -> min(10.0, 1.0) = 1.0, baseWeight=5 -> 1.0 + // Critical health: fixed 1.0 recovery weight for all enabled targets. if healthScore <= m.config.CriticalHealthThreshold { - minWeight := float64(baseWeight) * 0.1 - if minWeight > 1.0 { - minWeight = 1.0 // Cap at 1.0 to ensure fair competition with healthy low-weight targets - } - if minWeight < 1.0 { - minWeight = 1.0 // Floor at 1.0 to keep enabled targets recoverable. - } - effectiveWeight = minWeight + effectiveWeight = 1.0 } else if healthScore < m.config.MediumHealthThreshold {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/services/dynamic_weight.go` around lines 653 - 663, The critical-health branch using minWeight := float64(baseWeight) * 0.1 then capping/flooring always results in 1.0, leaving the 10% calculation dead; update the branch handling when healthScore <= m.config.CriticalHealthThreshold to assign an explicit recovery weight of 1.0 (replace the minWeight calculation and both conditional adjustments with a single assignment minWeight = 1.0) so enabled targets have a clear, explicit recoverable weight; change references in this block that use minWeight accordingly (symbols: healthScore, m.config.CriticalHealthThreshold, baseWeight, minWeight).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/services/dynamic_weight.go`:
- Around line 653-663: The critical-health branch using minWeight :=
float64(baseWeight) * 0.1 then capping/flooring always results in 1.0, leaving
the 10% calculation dead; update the branch handling when healthScore <=
m.config.CriticalHealthThreshold to assign an explicit recovery weight of 1.0
(replace the minWeight calculation and both conditional adjustments with a
single assignment minWeight = 1.0) so enabled targets have a clear, explicit
recoverable weight; change references in this block that use minWeight
accordingly (symbols: healthScore, m.config.CriticalHealthThreshold, baseWeight,
minWeight).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c21cf4e4-5382-403c-b7ba-0bf97b60019b
📒 Files selected for processing (5)
internal/models/types.gointernal/services/dynamic_weight.gointernal/services/dynamic_weight_test.goweb/src/components/keys/AddSubGroupModal.vueweb/src/utils/display.ts
✅ Files skipped from review due to trivial changes (1)
- web/src/utils/display.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/models/types.go
- 将动态权重临界健康度分支改为显式固定 1.0 恢复权重 - 同步更新相关英文注释，移除旧 10% 权重语义
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/config/system_settings.go (1)
37-50: ⚡ Quick winConsider adding clarifying comments to normalization logic.
The normalization logic correctly ensures
RequestTimeoutandNonStreamRequestTimeoutstay synchronized, withNonStreamRequestTimeoutas the source of truth. However, the control flow is somewhat non-obvious:
- Lines 41-43 backfill
NonStreamRequestTimeoutfrom legacy when only legacy exists- Lines 44-47 always sync legacy from non-stream when non-stream exists
- Line 49 syncs when neither flag is set (both are defaults)
Adding a brief comment explaining the invariant ("ensures RequestTimeout == NonStreamRequestTimeout with NonStreamRequestTimeout as source of truth") would improve maintainability.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/config/system_settings.go` around lines 37 - 50, Add a brief clarifying comment at the top of normalizeSplitRequestTimeouts explaining the invariant: RequestTimeout and NonStreamRequestTimeout are kept in sync with NonStreamRequestTimeout as the source of truth; mention the three cases handled (legacy-only backfill, non-stream present forcing sync, and default sync when neither flag set). Place this comment immediately above the normalizeSplitRequestTimeouts function definition so future readers see the intended behavior before the control flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/config/system_settings.go`:
- Around line 37-50: Add a brief clarifying comment at the top of
normalizeSplitRequestTimeouts explaining the invariant: RequestTimeout and
NonStreamRequestTimeout are kept in sync with NonStreamRequestTimeout as the
source of truth; mention the three cases handled (legacy-only backfill,
non-stream present forcing sync, and default sync when neither flag set). Place
this comment immediately above the normalizeSplitRequestTimeouts function
definition so future readers see the intended behavior before the control flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7ead2261-4664-4f02-9fbd-3b33d1e3be84
📒 Files selected for processing (50)
internal/app/app.gointernal/app/app_test.gointernal/channel/base_channel_test.gointernal/channel/factory.gointernal/channel/factory_test.gointernal/config/system_settings.gointernal/config/system_settings_test.gointernal/container/container.gointernal/db/migrations/migration.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.gointernal/db/migrations/v1_27_0_AddProxyPoolNameUniqueIndex.gointernal/db/migrations/v1_27_0_AddProxyPoolNameUniqueIndex_test.gointernal/handler/handler.gointernal/handler/proxy_pool_handler.gointernal/handler/proxy_pool_handler_test.gointernal/handler/test_main_test.gointernal/httpclient/manager.gointernal/i18n/locales/en-US.gointernal/i18n/locales/ja-JP.gointernal/i18n/locales/locales_test.gointernal/i18n/locales/zh-CN.gointernal/models/types.gointernal/proxy/cc_support.gointernal/proxy/cc_support_test.gointernal/proxy/server.gointernal/proxy/server_test.gointernal/router/router.gointernal/services/dynamic_weight.gointernal/services/dynamic_weight_test.gointernal/services/group_service.gointernal/services/group_service_test.gointernal/services/proxy_pool_service.gointernal/services/proxy_pool_service_test.gointernal/types/types.gointernal/utils/config_utils.gointernal/utils/proxy_url.gointernal/utils/proxy_url_test.goweb/src/api/proxy-pool.tsweb/src/components/NavBar.vueweb/src/components/keys/AddSubGroupModal.vueweb/src/components/keys/GroupFormModal.vueweb/src/features/proxy-pool/components/ProxyPoolPanel.vueweb/src/locales/en-US.tsweb/src/locales/ja-JP.tsweb/src/locales/zh-CN.tsweb/src/types/models.tsweb/src/utils/display.tsweb/src/views/More.vueweb/src/views/Settings.vue
为 normalizeSplitRequestTimeouts 增加入口级英文注释，说明 legacy request_timeout 回填、显式 non_stream_request_timeout 同步和默认同步三种分支。 本次不改运行逻辑，仅提升 PR #131 最新 CodeRabbit 审核关注点的可维护性。
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
internal/config/system_settings.go (1)
37-54: 💤 Low valueConsider simplifying the control flow.
The normalization logic is correct but has a redundant final assignment at line 53. When
hasNonStreamis false and the function reaches line 53, either:
hasLegacywas true andNonStreamRequestTimeoutwas already backfilled fromRequestTimeoutat line 46, or- Both are false and both fields are at their defaults
In both cases, syncing them again is a no-op. Consider restructuring for clarity:
♻️ Optional refactor for clarity
func normalizeSplitRequestTimeouts(settings *types.SystemSettings, hasLegacy, hasNonStream bool) { if settings == nil { return } - if hasLegacy && !hasNonStream { - settings.NonStreamRequestTimeout = settings.RequestTimeout - } if hasNonStream { - // Explicit zero disables non-stream timeout; keep legacy fallback synced to the same value. + // Non-stream is source of truth; sync legacy to match (including explicit zero). settings.RequestTimeout = settings.NonStreamRequestTimeout - return + } else if hasLegacy { + // Legacy-only: backfill non-stream from legacy. + settings.NonStreamRequestTimeout = settings.RequestTimeout + settings.RequestTimeout = settings.NonStreamRequestTimeout + } else { + // Neither provided: ensure they stay synced at defaults. + settings.RequestTimeout = settings.NonStreamRequestTimeout } - settings.RequestTimeout = settings.NonStreamRequestTimeout }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/config/system_settings.go` around lines 37 - 54, The function normalizeSplitRequestTimeouts has a redundant final assignment syncing settings.RequestTimeout = settings.NonStreamRequestTimeout; simplify control flow by making the three cases explicit: (1) if settings is nil return; (2) if hasLegacy && !hasNonStream backfill settings.NonStreamRequestTimeout = settings.RequestTimeout; (3) if hasNonStream set settings.RequestTimeout = settings.NonStreamRequestTimeout and return; otherwise do nothing (omit the final assignment). Update normalizeSplitRequestTimeouts to remove the no-op assignment and leave the defaults untouched, referencing the existing parameters settings, hasLegacy, hasNonStream and the fields RequestTimeout and NonStreamRequestTimeout.internal/db/migrations/v1_27_0_AddProxyPoolNameUniqueIndex_test.go (1)
69-89: ⚡ Quick winAdd
t.Parallel()for consistency and faster test execution.Tests 1 and 2 are marked parallel, but this test is not. Since all three tests use independent in-memory SQLite databases with no shared state, this test should also be parallel.
🔧 Suggested addition
func TestV1_27_0_AddProxyPoolNameUniqueIndexSkipsWhenMarkerAlreadyAcquired(t *testing.T) { + t.Parallel() + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/db/migrations/v1_27_0_AddProxyPoolNameUniqueIndex_test.go` around lines 69 - 89, The test TestV1_27_0_AddProxyPoolNameUniqueIndexSkipsWhenMarkerAlreadyAcquired is not running in parallel while sibling tests are; add t.Parallel() as the first statement in that test function to enable parallel execution (since it uses an independent in-memory SQLite DB) to match the other tests and speed up the suite; no other behaviour changes needed and the migration call V1_27_0_AddProxyPoolNameUniqueIndex can remain unchanged.internal/handler/proxy_pool_handler.go (2)
15-22: ⚡ Quick winPreserve the original error message in the fallback case.
When the service returns an error that isn't already an
*app_errors.APIError, the helper discards the original error detail and returns the generic message "unexpected proxy pool error". This makes debugging harder when unexpected errors occur.📝 Suggested improvement
func respondProxyPoolServiceError(c *gin.Context, err error) { var apiErr *app_errors.APIError if errors.As(err, &apiErr) { response.Error(c, apiErr) return } - response.Error(c, app_errors.NewAPIError(app_errors.ErrInternalServer, "unexpected proxy pool error")) + response.Error(c, app_errors.NewAPIError(app_errors.ErrInternalServer, err.Error())) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/handler/proxy_pool_handler.go` around lines 15 - 22, The fallback in respondProxyPoolServiceError drops the original error text; update it so when errors.As check fails you create and return an API error that includes the original err.Error() (e.g. pass fmt.Sprintf("unexpected proxy pool error: %s", err.Error()) or wrap the error) instead of the hardcoded string, using app_errors.NewAPIError and response.Error to preserve original error details for debugging.
61-64: ⚡ Quick winExtract the duplicated ID parsing and validation into a helper function.
The ID parsing and validation logic at lines 61-64 (Update), 84-87 (Delete), and 98-101 (Test) is identical. Extracting it into a shared helper improves maintainability and reduces the risk of inconsistent validation logic.
♻️ Suggested refactor
Add a helper function to the file:
// parseProxyPoolID parses and validates the proxy pool ID from the route parameter. // Returns the parsed ID and an APIError if parsing fails or the ID is invalid. func parseProxyPoolID(c *gin.Context) (uint, *app_errors.APIError) { id, err := strconv.Atoi(c.Param("id")) if err != nil || id <= 0 { return 0, app_errors.NewAPIError(app_errors.ErrBadRequest, "invalid proxy ID") } return uint(id), nil }Then replace the three occurrences:
func (s *Server) UpdateProxyPoolItem(c *gin.Context) { - id, err := strconv.Atoi(c.Param("id")) - if err != nil || id <= 0 { - response.Error(c, app_errors.NewAPIError(app_errors.ErrBadRequest, "invalid proxy ID")) + id, apiErr := parseProxyPoolID(c) + if apiErr != nil { + response.Error(c, apiErr) return }Apply the same pattern to
DeleteProxyPoolItemandTestProxyPoolItem.Also applies to: 84-87, 98-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/handler/proxy_pool_handler.go` around lines 61 - 64, Duplicate ID parsing/validation exists in UpdateProxyPoolItem, DeleteProxyPoolItem, and TestProxyPoolItem; extract it into a helper func parseProxyPoolID(c *gin.Context) (uint, *app_errors.APIError) that performs strconv.Atoi on c.Param("id"), checks id>0, and returns a uint ID or an APIError, then replace the duplicated blocks in UpdateProxyPoolItem, DeleteProxyPoolItem and TestProxyPoolItem to call parseProxyPoolID and handle the returned APIError (respond and return) before proceeding.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/config/system_settings.go`:
- Around line 37-54: The function normalizeSplitRequestTimeouts has a redundant
final assignment syncing settings.RequestTimeout =
settings.NonStreamRequestTimeout; simplify control flow by making the three
cases explicit: (1) if settings is nil return; (2) if hasLegacy && !hasNonStream
backfill settings.NonStreamRequestTimeout = settings.RequestTimeout; (3) if
hasNonStream set settings.RequestTimeout = settings.NonStreamRequestTimeout and
return; otherwise do nothing (omit the final assignment). Update
normalizeSplitRequestTimeouts to remove the no-op assignment and leave the
defaults untouched, referencing the existing parameters settings, hasLegacy,
hasNonStream and the fields RequestTimeout and NonStreamRequestTimeout.
In `@internal/db/migrations/v1_27_0_AddProxyPoolNameUniqueIndex_test.go`:
- Around line 69-89: The test
TestV1_27_0_AddProxyPoolNameUniqueIndexSkipsWhenMarkerAlreadyAcquired is not
running in parallel while sibling tests are; add t.Parallel() as the first
statement in that test function to enable parallel execution (since it uses an
independent in-memory SQLite DB) to match the other tests and speed up the
suite; no other behaviour changes needed and the migration call
V1_27_0_AddProxyPoolNameUniqueIndex can remain unchanged.
In `@internal/handler/proxy_pool_handler.go`:
- Around line 15-22: The fallback in respondProxyPoolServiceError drops the
original error text; update it so when errors.As check fails you create and
return an API error that includes the original err.Error() (e.g. pass
fmt.Sprintf("unexpected proxy pool error: %s", err.Error()) or wrap the error)
instead of the hardcoded string, using app_errors.NewAPIError and response.Error
to preserve original error details for debugging.
- Around line 61-64: Duplicate ID parsing/validation exists in
UpdateProxyPoolItem, DeleteProxyPoolItem, and TestProxyPoolItem; extract it into
a helper func parseProxyPoolID(c *gin.Context) (uint, *app_errors.APIError) that
performs strconv.Atoi on c.Param("id"), checks id>0, and returns a uint ID or an
APIError, then replace the duplicated blocks in UpdateProxyPoolItem,
DeleteProxyPoolItem and TestProxyPoolItem to call parseProxyPoolID and handle
the returned APIError (respond and return) before proceeding.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90bfd847-0af2-4ee3-ae34-9924ce63c29a
📒 Files selected for processing (50)
internal/app/app.gointernal/app/app_test.gointernal/channel/base_channel_test.gointernal/channel/factory.gointernal/channel/factory_test.gointernal/config/system_settings.gointernal/config/system_settings_test.gointernal/container/container.gointernal/db/migrations/migration.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.gointernal/db/migrations/v1_27_0_AddProxyPoolNameUniqueIndex.gointernal/db/migrations/v1_27_0_AddProxyPoolNameUniqueIndex_test.gointernal/handler/handler.gointernal/handler/proxy_pool_handler.gointernal/handler/proxy_pool_handler_test.gointernal/handler/test_main_test.gointernal/httpclient/manager.gointernal/i18n/locales/en-US.gointernal/i18n/locales/ja-JP.gointernal/i18n/locales/locales_test.gointernal/i18n/locales/zh-CN.gointernal/models/types.gointernal/proxy/cc_support.gointernal/proxy/cc_support_test.gointernal/proxy/server.gointernal/proxy/server_test.gointernal/router/router.gointernal/services/dynamic_weight.gointernal/services/dynamic_weight_test.gointernal/services/group_service.gointernal/services/group_service_test.gointernal/services/proxy_pool_service.gointernal/services/proxy_pool_service_test.gointernal/types/types.gointernal/utils/config_utils.gointernal/utils/proxy_url.gointernal/utils/proxy_url_test.goweb/src/api/proxy-pool.tsweb/src/components/NavBar.vueweb/src/components/keys/AddSubGroupModal.vueweb/src/components/keys/GroupFormModal.vueweb/src/features/proxy-pool/components/ProxyPoolPanel.vueweb/src/locales/en-US.tsweb/src/locales/ja-JP.tsweb/src/locales/zh-CN.tsweb/src/types/models.tsweb/src/utils/display.tsweb/src/views/More.vueweb/src/views/Settings.vue
简化 split timeout 归一化分支，保留 legacy 与 non_stream_request_timeout 的兼容语义。 代理池 handler 复用 ID 解析逻辑；未知服务错误仅记录服务端日志，避免向客户端泄漏底层细节。 代理池唯一索引迁移测试补充安全的并行执行。
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/services/dynamic_weight.go (1)
145-147:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale recovery-floor comment to match current behavior.
Line 145 still says
(min 0.1), butGetEffectiveWeightnow enforces a 1.0 minimum for enabled targets (Lines 668-670). Please align this comment to avoid future confusion.Suggested diff
- // Below this, effective weight is capped at 1 (min 0.1) to prevent unhealthy + // Below this, effective weight is capped at 1.0 (minimum recovery floor) to prevent unhealthy🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/services/dynamic_weight.go` around lines 145 - 147, Update the stale comment that mentions "(min 0.1)" near CriticalHealthThreshold to reflect the current minimum enforced by GetEffectiveWeight (which enforces a 1.0 minimum for enabled targets). Locate the comment block containing CriticalHealthThreshold and change the text to indicate that effective weight is capped at 1.0 (not 0.1) for enabled targets, keeping the rest of the explanatory context intact so it matches the logic in GetEffectiveWeight.
🧹 Nitpick comments (2)
web/src/features/proxy-pool/components/ProxyPoolPanel.vue (1)
553-572: 💤 Low valueForm validation error can leave
savingstuck asfalse.If
formRef.value?.validate()throws a validation error, execution stops beforesaving.value = true, so the finally block correctly handles that case. However, if the API call fails, there's no error feedback to the user beyond the global http error handler.Consider adding explicit error handling for better UX consistency with other operations in this component.
💡 Optional improvement for explicit error handling
async function submit() { await formRef.value?.validate(); saving.value = true; try { const payload = { name: form.name.trim(), url: form.url.trim() }; const creating = !editingItem.value; if (editingItem.value) { await proxyPoolApi.update(editingItem.value.id, payload); } else { await proxyPoolApi.create(payload); } showModal.value = false; resetForm(); if (creating) { currentPage.value = 1; } await loadItems(); + } catch { + message.error(t("common.operationFailed")); } finally { saving.value = false; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/features/proxy-pool/components/ProxyPoolPanel.vue` around lines 553 - 572, The submit function can leave the user without explicit feedback if proxyPoolApi.create/update fails; add a try/catch around the awaited API call(s) inside submit to catch errors from proxyPoolApi.create/update, set saving.value appropriately, show a user-facing error notification or set a local error state (use the same UX pattern used elsewhere in this component), and only run the success path (showModal.value = false, resetForm(), currentPage adjustment, loadItems()) on success; keep the existing finally block to always set saving.value = false. Target the submit function and the references to formRef, saving, proxyPoolApi.create/update, showModal, resetForm, currentPage, and loadItems.internal/i18n/locales/locales_test.go (1)
187-200: ⚡ Quick winInclude
*_desckeys in the required config-key set.This test currently protects new labels but not their description keys, so a cross-locale removal of descriptions can slip through while tests stay green. Adding the new timeout/proxy-pool
*_desckeys here will lock this down.Proposed diff
configKeys := []string{ "config.updated", "config.app_url", "config.proxy_keys", "config.log_retention_days", "config.request_timeout", + "config.request_timeout_desc", "config.non_stream_request_timeout", + "config.non_stream_request_timeout_desc", "config.stream_request_timeout", + "config.stream_request_timeout_desc", "config.proxy_pool_test_target_url", + "config.proxy_pool_test_target_url_desc", "config.proxy_pool_test_timeout_seconds", + "config.proxy_pool_test_timeout_seconds_desc", "config.proxy_pool_auto_test_interval_minutes", + "config.proxy_pool_auto_test_interval_minutes_desc", "config.category.proxy_pool", "config.max_retries", }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/i18n/locales/locales_test.go` around lines 187 - 200, The test's configKeys slice (in locales_test.go) only validates label keys and misses the corresponding description keys, allowing description removals to go unnoticed; update the configKeys variable to include each "*_desc" counterpart for entries that have descriptions (e.g., add "config.request_timeout_desc", "config.non_stream_request_timeout_desc", "config.stream_request_timeout_desc", "config.proxy_pool_test_timeout_seconds_desc", "config.proxy_pool_auto_test_interval_minutes_desc", "config.proxy_pool_test_target_url_desc" and any other matching description keys) so that the test enforces both label and description presence for functions/keys that expect descriptions. Ensure the added strings match the naming pattern used by other keys in the slice.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/models/types.go`:
- Around line 31-35: The ProxyPoolItem struct persists full proxy URLs in
ProxyPoolItem.URL which may contain plaintext credentials; change the model to
separate endpoint vs secret material (e.g., add Endpoint and an encrypted
Credentials/EncryptedAuth field or UsernameEncrypted and PasswordEncrypted) and
remove or omit raw credential-containing fields from JSON (update struct tags to
not serialize secrets). Implement setter/getter helpers on ProxyPoolItem (e.g.,
SetProxyCredentials, BuildProxyURL, DecryptCredentials) that encrypt/decrypt
secrets using the project’s crypto key management, persist only encrypted bytes,
and ensure any read/JSON serialization returns either the endpoint or a masked
URL (e.g., user:****`@host`) instead of raw credentials. Update any code paths
that write/read ProxyPoolItem (create/update APIs and admin views) to use the
new setters and to never log or serialize plaintext credentials.
---
Outside diff comments:
In `@internal/services/dynamic_weight.go`:
- Around line 145-147: Update the stale comment that mentions "(min 0.1)" near
CriticalHealthThreshold to reflect the current minimum enforced by
GetEffectiveWeight (which enforces a 1.0 minimum for enabled targets). Locate
the comment block containing CriticalHealthThreshold and change the text to
indicate that effective weight is capped at 1.0 (not 0.1) for enabled targets,
keeping the rest of the explanatory context intact so it matches the logic in
GetEffectiveWeight.
---
Nitpick comments:
In `@internal/i18n/locales/locales_test.go`:
- Around line 187-200: The test's configKeys slice (in locales_test.go) only
validates label keys and misses the corresponding description keys, allowing
description removals to go unnoticed; update the configKeys variable to include
each "*_desc" counterpart for entries that have descriptions (e.g., add
"config.request_timeout_desc", "config.non_stream_request_timeout_desc",
"config.stream_request_timeout_desc",
"config.proxy_pool_test_timeout_seconds_desc",
"config.proxy_pool_auto_test_interval_minutes_desc",
"config.proxy_pool_test_target_url_desc" and any other matching description
keys) so that the test enforces both label and description presence for
functions/keys that expect descriptions. Ensure the added strings match the
naming pattern used by other keys in the slice.
In `@web/src/features/proxy-pool/components/ProxyPoolPanel.vue`:
- Around line 553-572: The submit function can leave the user without explicit
feedback if proxyPoolApi.create/update fails; add a try/catch around the awaited
API call(s) inside submit to catch errors from proxyPoolApi.create/update, set
saving.value appropriately, show a user-facing error notification or set a local
error state (use the same UX pattern used elsewhere in this component), and only
run the success path (showModal.value = false, resetForm(), currentPage
adjustment, loadItems()) on success; keep the existing finally block to always
set saving.value = false. Target the submit function and the references to
formRef, saving, proxyPoolApi.create/update, showModal, resetForm, currentPage,
and loadItems.
🪄 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: 556ab46d-e8af-46ed-99f4-92fcb5e901fa
📒 Files selected for processing (50)
internal/app/app.gointernal/app/app_test.gointernal/channel/base_channel_test.gointernal/channel/factory.gointernal/channel/factory_test.gointernal/config/system_settings.gointernal/config/system_settings_test.gointernal/container/container.gointernal/db/migrations/migration.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs.gointernal/db/migrations/v1_26_0_ClearLegacyProxyURLs_test.gointernal/db/migrations/v1_27_0_AddProxyPoolNameUniqueIndex.gointernal/db/migrations/v1_27_0_AddProxyPoolNameUniqueIndex_test.gointernal/handler/handler.gointernal/handler/proxy_pool_handler.gointernal/handler/proxy_pool_handler_test.gointernal/handler/test_main_test.gointernal/httpclient/manager.gointernal/i18n/locales/en-US.gointernal/i18n/locales/ja-JP.gointernal/i18n/locales/locales_test.gointernal/i18n/locales/zh-CN.gointernal/models/types.gointernal/proxy/cc_support.gointernal/proxy/cc_support_test.gointernal/proxy/server.gointernal/proxy/server_test.gointernal/router/router.gointernal/services/dynamic_weight.gointernal/services/dynamic_weight_test.gointernal/services/group_service.gointernal/services/group_service_test.gointernal/services/proxy_pool_service.gointernal/services/proxy_pool_service_test.gointernal/types/types.gointernal/utils/config_utils.gointernal/utils/proxy_url.gointernal/utils/proxy_url_test.goweb/src/api/proxy-pool.tsweb/src/components/NavBar.vueweb/src/components/keys/AddSubGroupModal.vueweb/src/components/keys/GroupFormModal.vueweb/src/features/proxy-pool/components/ProxyPoolPanel.vueweb/src/locales/en-US.tsweb/src/locales/ja-JP.tsweb/src/locales/zh-CN.tsweb/src/types/models.tsweb/src/utils/display.tsweb/src/views/More.vueweb/src/views/Settings.vue
Comment on lines
+31
to
+35
| type ProxyPoolItem struct { | ||
| ID uint `gorm:"primaryKey;autoIncrement" json:"id"` | ||
| Name string `gorm:"type:varchar(255);not null;uniqueIndex:idx_proxy_pool_items_name_unique" json:"name"` | ||
| URL string `gorm:"type:text;not null" json:"url"` | ||
| CreatedAt time.Time `json:"created_at"` |
There was a problem hiding this comment.
Avoid persisting proxy credentials in plaintext.
ProxyPoolItem.URL stores the full proxy URL as raw text and is also serialized directly. Since proxy URLs commonly embed user:pass@, this exposes credentials at rest and in API payloads/admin views. Persist encrypted secret material (or split endpoint vs credentials and encrypt secrets), and only return masked URLs on read paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/models/types.go` around lines 31 - 35, The ProxyPoolItem struct
persists full proxy URLs in ProxyPoolItem.URL which may contain plaintext
credentials; change the model to separate endpoint vs secret material (e.g., add
Endpoint and an encrypted Credentials/EncryptedAuth field or UsernameEncrypted
and PasswordEncrypted) and remove or omit raw credential-containing fields from
JSON (update struct tags to not serialize secrets). Implement setter/getter
helpers on ProxyPoolItem (e.g., SetProxyCredentials, BuildProxyURL,
DecryptCredentials) that encrypt/decrypt secrets using the project’s crypto key
management, persist only encrypted bytes, and ensure any read/JSON serialization
returns either the endpoint or a masked URL (e.g., user:****`@host`) instead of
raw credentials. Update any code paths that write/read ProxyPoolItem
(create/update APIs and admin views) to use the new setters and to never log or
serialize plaintext credentials.
更新动态权重临界健康度注释，并补齐配置项描述 key 的跨语言测试覆盖。 代理池新增/编辑失败时补充显式错误提示；代理池列表 JSON 输出会脱敏 URL 凭据，编辑脱敏后的同 endpoint URL 时保留已存认证信息，避免只改名称导致代理凭据丢失。
This was referenced Jun 12, 2026
Merged
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.
新增更多-代理池，支持维护 http/https/socks5 代理，并将系统设置和标准分组上游代理改为从代理池选择；升级迁移会一次性清空旧代理配置，后续重建代理池后不再重复清空。
代理池支持手动测试、检测全部和默认 1 小时的页面内定期检测，单个代理显示 10 秒测试超时，检测错误会脱敏代理凭据；代理新增/编辑弹框改为与现有表单弹框一致的卡片式布局。
拆分非流式/流式请求超时配置，并调整标准分组与聚合分组超时后的重试行为；补充代理池保存、分组代理保存、代理流转、迁移幂等性、超时重试和代理检测相关测试。
关联 Issue / Related Issue
Closes #
变更内容 / Change Content
自查清单 / Checklist
Summary by CodeRabbit
New Features
New Configuration / Behavior
Bug Fixes / Data Migration
Tests
Localization