Skip to content

feat(proxy): add admin kill switch and expose it in routes page#518

Open
awsl233777 wants to merge 3 commits into
mainfrom
fix/proxy-kill-switch-routes-page
Open

feat(proxy): add admin kill switch and expose it in routes page#518
awsl233777 wants to merge 3 commits into
mainfrom
fix/proxy-kill-switch-routes-page

Conversation

@awsl233777
Copy link
Copy Markdown
Collaborator

@awsl233777 awsl233777 commented Apr 14, 2026

Summary

  • add a proxy request kill switch system setting that blocks all new proxy requests with HTTP 503
  • validate the setting server-side and cover the behavior with handler/service tests
  • expose the same kill switch card in both the Settings page and the client Routes page so operators can toggle it where they manage routing

Testing

  • go test ./internal/handler ./internal/service
  • cd web && pnpm typecheck

Summary by CodeRabbit

发布说明

  • 新功能

    • 管理端新增“代理请求总闸”设置卡片,支持一键启用/禁用所有代理请求(启用时返回 503)。
    • 新增中英界面文案与开关可见性仅限管理员。
  • 改进

    • 运行时即时遵循总闸开关,拒绝被禁用状态下的代理请求。
    • 新增布尔型系统设置验证与本地缓存,并在设置变更后自动失效以确保一致性。
  • 测试

    • 增加端到端与单元测试,覆盖开关行为、输入校验、缓存命中与回退场景。

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 14, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

将全局代理请求“总闸”引入代理处理链:注入 system setting 仓库到 ProxyHandler,入站首先读取并缓存 proxy_requests_disabled;若为 true 则记录并返回 503;同时新增设置常量、严格验证、布尔缓存与管理端失效,及管理员前端开关与 i18n 支持。

Changes

后端依赖注入

Layer / File(s) Summary
构造时注入 SettingRepo
cmd/maxx/main.go, internal/core/database.go, tests/e2e/proxy_setup_test.go
调整 handler.NewProxyHandler 调用,新增传入 settingRepo 参数以将系统设置仓库注入 ProxyHandler。

代理处理与测试

Layer / File(s) Summary
ProxyHandler 逻辑与单元测试
internal/handler/proxy.go, internal/handler/proxy_test.go
为 ProxyHandler 注入 settingRepo;在 ingress 开头通过 systemsettingcache.GetBoolean 检查 proxy_requests_disabled,启用时写 503 并中止;新增相关测试桩与用例覆盖缓存命中、刷新回退与拒绝行为。

系统设置缓存与验证

Layer / File(s) Summary
布尔设置缓存与测试
internal/systemsettingcache/boolean.go, internal/systemsettingcache/boolean_test.go
新增内存布尔缓存(可配置 BooleanTTL)、GetBoolean/Invalidate 实现;在 repo 读取失败时回退到上次已知值;包含缓存命中与回退场景的测试。
系统设置常量与验证
internal/domain/model.go, internal/service/system_setting_validation.go, internal/service/system_setting_validation_test.go
新增 SettingKeyProxyRequestsDisabled 常量;为该键添加严格布尔验证(仅接受精确 "true"/"false",拒绝首尾空白),并增加测试覆盖;测试还验证更新/删除后缓存读值与失效行为。
Admin 服务缓存失效
internal/service/admin.go
AdminService.UpdateSetting / DeleteSetting 成功后调用 systemsettingcache.Invalidate(key) 以使缓存同步最新值。

前端与本地化

Layer / File(s) Summary
前端设置与页面集成
web/src/components/settings/proxy-kill-switch-card.tsx, web/src/pages/settings/index.tsx, web/src/pages/client-routes/index.tsx
新增管理员可见的 ProxyKillSwitchCard,在设置页与客户端路由(仅 admin)集成;切换通过 API 更新 proxy_requests_disabled
国际化文案
web/src/locales/en.json, web/src/locales/zh.json
新增用于代理总闸的 i18n 文案键:开关标题、描述、启用提示与启用时的警示文本。

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant ProxyHandler
    participant SettingCache as systemsettingcache
    participant SettingRepo
    participant Executor

    Client->>ProxyHandler: 发送代理请求
    ProxyHandler->>SettingCache: GetBoolean(repos.SettingRepo, "proxy_requests_disabled")
    SettingCache->>SettingRepo: repo.Get("proxy_requests_disabled") (当缓存过期)
    SettingRepo-->>SettingCache: 返回字符串值或错误
    alt 设置为 "true"
        ProxyHandler->>Client: 返回 503 Service Unavailable(proxyRequestsDisabledMessage)
    else 设置为 "false" 或 不存在 / 缓存回退
        ProxyHandler->>Executor: 转发并执行代理请求
        Executor-->>ProxyHandler: 返回执行结果
        ProxyHandler->>Client: 返回代理响应
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • awsl-project/maxx#454: 修改了 internal/handler/proxy.go 的 ingress 流,触及相同处理路径与逻辑位置。

Suggested reviewers

  • ymkiux
  • solarhell

Poem

🐰 我这只小兔说:
总闸一按风停船,
前端轻摇后端谨。
管理页上红警示,
请求门外静且安,
胡萝卜与代码同欢 🎋

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题清晰准确地反映了本次变更的核心内容:为代理请求添加管理员杀死开关并在路由页面中暴露该功能。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/proxy-kill-switch-routes-page

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
internal/handler/proxy_test.go (1)

144-149: 建议补充“大小写/空白”解析用例,防止语义漂移。

Line [144] 目前只覆盖“缺失值默认 false”。建议再加一个值为 " TRUE " 的用例,和后端 TrimSpace + EqualFold 语义保持显式绑定。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/handler/proxy_test.go` around lines 144 - 149, Add a complementary
test case to TestIsBooleanSystemSettingEnabledDefaultsFalse that verifies
isBooleanSystemSettingEnabled correctly handles surrounding whitespace and
case-insensitivity (i.e., a stored value of " TRUE "); create or reuse
settingsTestRepo to return the string " TRUE " for
domain.SettingKeyProxyRequestsDisabled and assert the function returns true,
ensuring behavior matches TrimSpace + EqualFold semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/handler/proxy.go`:
- Around line 323-335: The isBooleanSystemSettingEnabled function currently
calls repo.Get on every request (repo.Get) which causes DB pressure and treats
any read error as false; change it to use a short-TTL in-process cache keyed by
the setting key and store both the cached value and the timestamp (e.g., a map
guarded by sync.RWMutex or a concurrent safe structure), have
isBooleanSystemSettingEnabled first consult the cache and return the cached
value if TTL not expired, when TTL expired fetch repo.Get(key) to refresh the
cache and update the last-known value, and crucially if repo.Get returns an
error keep and return the most-recent cached value instead of falling back to
false so transient repo failures do not flip the feature off; ensure the cache
is goroutine-safe and TTL configurable.

In `@internal/service/system_setting_validation.go`:
- Around line 23-27: The validation currently uses strings.TrimSpace(value)
which allows values like " true " — change it to reject any value with
leading/trailing whitespace and only accept exact "true" or "false". In the
switch block (where strings.TrimSpace(value) is used), first check if
strings.TrimSpace(value) != value and return domain.ErrInvalidInput with a
message about no surrounding whitespace allowed; then only allow the exact
literals "true" or "false" (using the variable value itself) and return the
existing fmt.Errorf("%w: boolean setting must be \"true\" or \"false\"",
domain.ErrInvalidInput) on mismatch.

In `@web/src/components/settings/proxy-kill-switch-card.tsx`:
- Line 16: The current boolean parsing for proxyRequestsDisabled only checks
strict equality to 'true' and will miss variants like ' TRUE ' or different
casing; update the evaluation of settings.proxy_requests_disabled in the
proxyRequestsDisabled assignment to normalize the value (e.g., coerce to string,
trim whitespace and lowercase) before comparing to 'true' so values like " TRUE
" or "True" correctly resolve; reference the proxyRequestsDisabled constant and
the settings.proxy_requests_disabled property when making this change.
- Around line 50-54: The Switch component controlling proxy requests (the
instance with props checked={proxyRequestsDisabled},
onCheckedChange={handleToggle}, disabled={updateSetting.isPending}) lacks an
accessible name; add an explicit accessible label by supplying an aria-label or
aria-labelledby (or connect a visible/visually-hidden <label>) that clearly
describes the switch purpose (e.g., "Disable proxy requests") so screen readers
can identify its function while keeping existing props and handlers unchanged.

---

Nitpick comments:
In `@internal/handler/proxy_test.go`:
- Around line 144-149: Add a complementary test case to
TestIsBooleanSystemSettingEnabledDefaultsFalse that verifies
isBooleanSystemSettingEnabled correctly handles surrounding whitespace and
case-insensitivity (i.e., a stored value of " TRUE "); create or reuse
settingsTestRepo to return the string " TRUE " for
domain.SettingKeyProxyRequestsDisabled and assert the function returns true,
ensuring behavior matches TrimSpace + EqualFold semantics.
🪄 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: 2ba1bba7-59ed-413d-91fd-26db60e46f7c

📥 Commits

Reviewing files that changed from the base of the PR and between 4c89d1e and 99f1364.

📒 Files selected for processing (13)
  • cmd/maxx/main.go
  • internal/core/database.go
  • internal/domain/model.go
  • internal/handler/proxy.go
  • internal/handler/proxy_test.go
  • internal/service/system_setting_validation.go
  • internal/service/system_setting_validation_test.go
  • tests/e2e/proxy_setup_test.go
  • web/src/components/settings/proxy-kill-switch-card.tsx
  • web/src/locales/en.json
  • web/src/locales/zh.json
  • web/src/pages/client-routes/index.tsx
  • web/src/pages/settings/index.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: e2e
  • GitHub Check: Frontend Checks
  • GitHub Check: Backend Checks
  • GitHub Check: playwright
🔇 Additional comments (11)
web/src/locales/zh.json (1)

957-960: 文案补充完整且语义清晰。

该段新增键覆盖了开关功能的核心交互与风险提示,适配管理场景良好。

web/src/pages/client-routes/index.tsx (1)

139-142: 管理员可见性控制实现正确。

Line 139-142 与 Line 263-269 的组合使熔断开关仅对 admin 暴露,页面集成方式也与现有布局一致。

Also applies to: 263-269

web/src/pages/settings/index.tsx (1)

44-44: 设置页集成位置合理,权限边界清晰。

开关卡片被纳入 admin 专属区块,功能入口与权限策略保持一致。

Also applies to: 288-288, 790-792

cmd/maxx/main.go (1)

397-397: 构造参数对齐正确。

Line 397 传入 settingRepo 后,代理处理器可读取系统级开关配置,初始化链路完整。

internal/service/system_setting_validation_test.go (1)

51-75: 新增测试覆盖点有效。

无效值拒绝与有效值持久化两条主路径都被断言,且包含“非法输入不应写入仓库”的检查。

tests/e2e/proxy_setup_test.go (1)

188-188: 测试环境构造参数已与主流程对齐。

Line 188 的更新保证 e2e 环境能覆盖系统设置驱动的代理行为(包括总闸)。

internal/domain/model.go (1)

648-648: 新系统设置键定义清晰且命名一致。

该常量可读性好,并与现有系统设置键风格保持统一。

internal/core/database.go (1)

423-423: 构造参数注入已对齐,改动正确。

这次把 repos.SettingRepo 注入到 NewProxyHandler 的位置正确,和 kill switch 逻辑的依赖一致。

web/src/locales/en.json (1)

958-961: 新增 i18n 键命名与语义一致,LGTM。

文案覆盖了开关名称、描述和启用提示,和后端行为对齐。

internal/handler/proxy_test.go (1)

127-142: 阻断行为测试覆盖到位。

用例验证了 kill switch 开启时返回 503 且包含预期提示,核心行为覆盖充分。

internal/handler/proxy.go (1)

112-117: 入口处提前拦截 kill switch,处理顺序合理。

在请求主流程前直接返回 503,避免进入后续鉴权/执行链,方向正确。

Comment thread internal/handler/proxy.go
Comment thread internal/service/system_setting_validation.go Outdated
Comment thread web/src/components/settings/proxy-kill-switch-card.tsx Outdated
Comment thread web/src/components/settings/proxy-kill-switch-card.tsx
@ymkiux
Copy link
Copy Markdown
Contributor

ymkiux commented Apr 16, 2026

@coderabbitai If the current suggestion is not adopted, will it lead to disruptive changes?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 16, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh pr view 518 --repo awsl-project/maxx --json reviews,comments
gh api repos/awsl-project/maxx/pulls/518/comments --jq '.[].body' 2>/dev/null | head -200
gh api repos/awsl-project/maxx/pulls/518/reviews --jq '.[].body' 2>/dev/null | head -200

Length of output: 39590


@ymkiux Here's a breakdown of each suggestion and its risk if not adopted:


1. 🟠 internal/handler/proxy.go — Per-request repo.Get without caching

Risk if not adopted: Medium-High (reliability concern under load)

Not adopting this won't cause an immediate crash or build failure. However:

  • Under high concurrency, every proxy request triggers a DB read, which amplifies database load and increases latency.
  • More critically, any transient repository error silently falls back to false, effectively disabling the kill switch without any alert. In an incident scenario where you've activated the kill switch to stop traffic, a brief DB hiccup would silently let requests through again — the opposite of the intended behavior.

This is not immediately disruptive, but it is a reliability risk in production.


2. 🟠 internal/service/system_setting_validation.go + proxy-kill-switch-card.tsx — Backend/frontend boolean parsing inconsistency

Risk if not adopted: Low in practice, but creates an operational pitfall

The validation accepts " true " (with spaces) after TrimSpace, while the frontend strictly checks === 'true'. If a padded value ever reaches the database (e.g., via a direct API call or curl), the backend would treat the kill switch as ON but the frontend would display it as OFF — operators would have no visible indication the switch is active.

In normal UI-driven usage, the frontend always sends exact "true"/"false" strings, so this inconsistency would not manifest. The risk is limited to non-UI API clients or manual DB manipulation.


3. 🟡 proxy-kill-switch-card.tsx — Missing aria-label on Switch

Risk if not adopted: None functionally

This is a pure accessibility issue. Screen reader users won't know what the switch controls. No functional or behavioral impact for sighted users.


Summary

Suggestion Hard Break? Production Risk
Add TTL cache + last-known-value fallback 🟠 Kill switch silently disabled during DB errors
Strict boolean validation (no whitespace) 🟡 State inconsistency via non-UI API clients only
Normalize frontend boolean parsing 🟡 Same as above (paired with validation fix)
Add aria-label to Switch ⬜ Accessibility only

The caching suggestion (#1) has the most meaningful production impact and is worth prioritizing, especially since this is an emergency kill switch intended for use during incidents.

@awsl233777
Copy link
Copy Markdown
Collaborator Author

已按这轮建议继续修复,并补了回归验证。

本次处理:

  • 为 kill switch 布尔读取加了短 TTL 进程内缓存,避免每个代理请求都打一次 settings repo
  • repo 读失败时回退到最近一次已知值,避免瞬时仓储抖动把 kill switch 静默放开
  • settings 更新/删除后主动失效缓存,避免缓存引入新的切换迟滞
  • 后端布尔设置改为拒绝首尾空白,消掉前后端状态语义不一致
  • 前端状态解析增加 trim/lowercase 归一化,并给 Switch 补了 aria-label
  • 补了缓存/失效/空白校验相关测试

已实际验证:

  • go test ./internal/systemsettingcache ./internal/handler ./internal/service
  • pnpm lint
  • pnpm typecheck

@coderabbitai re-review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 16, 2026

@awsl233777 感谢全面跟进!本轮所有改进点均已落地:

  • 短 TTL 进程内缓存 — 大幅降低 settings repo 的每请求读压力
  • read 失败时回退最近已知值 — 消除了仓储抖动导致 kill switch 被静默关闭的风险
  • settings 变更后主动失效缓存 — 保证切换操作实时生效,无迟滞
  • 后端拒绝首尾空白 — 消除前后端布尔语义不一致
  • 前端 trim/lowercase 归一化 + aria-label — 状态解析健壮性与可访问性均已覆盖
  • 补充回归测试 — 缓存/失效/空白校验均有对应用例,并经本地验证通过

即将触发完整 re-review。


🧠 Learnings used
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-30T09:23:52.570Z
Learning: In awsl-project/maxx PR `#466`, all 5 concerns raised in the initial review were incorrect. The `APITokenConcurrencySection` useEffect guard was present, `AcquireConcurrency` correctly returns `ErrInvalidToken` for zero-ID+empty-token, `ResolveToken` is fully tested, `IsStreamRequest` has no dependency on clientType, and passing `""` as ClientType in `ExtractToken` is intentional to scan all auth headers. Be more careful to read the actual code before flagging concerns.

Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-11T07:29:12.071Z
Learning: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
web/src/components/settings/proxy-kill-switch-card.tsx (1)

21-21: 建议将设置键提取为常量。

'proxy_requests_disabled' 在此处硬编码,建议提取为共享常量以便与其他使用该键的地方保持一致,减少拼写错误风险。

♻️ 建议修改
// 在 constants 或 types 文件中定义
export const SETTING_KEY_PROXY_REQUESTS_DISABLED = 'proxy_requests_disabled';

// 然后在组件中使用
import { SETTING_KEY_PROXY_REQUESTS_DISABLED } from '@/constants';
// ...
key: SETTING_KEY_PROXY_REQUESTS_DISABLED,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/settings/proxy-kill-switch-card.tsx` at line 21, The
hardcoded setting key 'proxy_requests_disabled' in the ProxyKillSwitchCard
component should be extracted to a shared constant to avoid duplication and
typos; create a new exported constant (e.g. SETTING_KEY_PROXY_REQUESTS_DISABLED)
in your existing constants/types module and replace the literal usage in
web/src/components/settings/proxy-kill-switch-card.tsx (the key:
'proxy_requests_disabled' entry) with an import of that constant so all callers
use the same identifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@web/src/components/settings/proxy-kill-switch-card.tsx`:
- Line 21: The hardcoded setting key 'proxy_requests_disabled' in the
ProxyKillSwitchCard component should be extracted to a shared constant to avoid
duplication and typos; create a new exported constant (e.g.
SETTING_KEY_PROXY_REQUESTS_DISABLED) in your existing constants/types module and
replace the literal usage in
web/src/components/settings/proxy-kill-switch-card.tsx (the key:
'proxy_requests_disabled' entry) with an import of that constant so all callers
use the same identifier.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8b13a897-e667-4c4d-9293-705d8cba9c2c

📥 Commits

Reviewing files that changed from the base of the PR and between 99f1364 and c5f043c.

📒 Files selected for processing (8)
  • internal/handler/proxy.go
  • internal/handler/proxy_test.go
  • internal/service/admin.go
  • internal/service/system_setting_validation.go
  • internal/service/system_setting_validation_test.go
  • internal/systemsettingcache/boolean.go
  • internal/systemsettingcache/boolean_test.go
  • web/src/components/settings/proxy-kill-switch-card.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • internal/service/system_setting_validation.go
  • internal/handler/proxy_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: playwright
  • GitHub Check: Frontend Checks
  • GitHub Check: Backend Checks
  • GitHub Check: e2e
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-30T09:23:52.570Z
Learning: In awsl-project/maxx PR `#466`, all 5 concerns raised in the initial review were incorrect. The `APITokenConcurrencySection` useEffect guard was present, `AcquireConcurrency` correctly returns `ErrInvalidToken` for zero-ID+empty-token, `ResolveToken` is fully tested, `IsStreamRequest` has no dependency on clientType, and passing `""` as ClientType in `ExtractToken` is intentional to scan all auth headers. Be more careful to read the actual code before flagging concerns.
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-11T07:29:12.071Z
Learning: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
🔇 Additional comments (11)
internal/handler/proxy.go (2)

320-326: 通过缓存层解决了热路径数据库访问问题。

之前评审中提出的每请求查库风险已通过 systemsettingcache.GetBoolean 解决。该缓存实现了:

  • TTL 控制的缓存刷新(默认 1 秒)
  • 刷新失败时回退到最近已知值
  • 管理员更新设置后立即失效缓存

113-118: Kill switch 检查位置合理。

在跟踪器检查和 HTTP 方法检查之后、请求体读取之前执行 kill switch 检查,确保:

  1. 优雅关闭逻辑不受影响
  2. 尽早拒绝请求,避免不必要的资源消耗
internal/service/admin.go (1)

475-475: 缓存失效逻辑位置正确。

在持久化成功后调用 systemsettingcache.Invalidate(key),确保并发读取不会重新缓存旧值。这与测试用例 TestAdminServiceUpdateSettingInvalidatesProxyBooleanCacheTestAdminServiceDeleteSettingInvalidatesProxyBooleanCache 的预期行为一致。

Also applies to: 497-497

internal/service/system_setting_validation_test.go (1)

53-132: 测试覆盖全面,验证了关键行为。

新增测试用例覆盖了:

  1. 拒绝无效布尔值("maybe"
  2. 接受有效布尔值("true"
  3. 拒绝带空白的值(" true "
  4. 更新设置后缓存失效
  5. 删除设置后缓存失效

测试正确使用 defer 恢复 BooleanTTL,并在断言前调用 Invalidate 确保缓存状态干净。

web/src/components/settings/proxy-kill-switch-card.tsx (2)

16-17: 布尔解析逻辑与后端保持一致。

前端使用 .trim().toLowerCase() === 'true' 与后端 strings.EqualFold(strings.TrimSpace(value), "true") 功能等价,解决了之前评审中提出的前后端不一致问题。


51-56: 无障碍标签已添加。

Switch 组件现在包含 aria-label={t('settings.enableProxyKillSwitch')},屏幕阅读器用户可以识别该控件的用途。

internal/systemsettingcache/boolean_test.go (2)

36-78: 测试覆盖了缓存的核心行为。

两个测试用例验证了:

  1. TestGetBooleanCachesFreshValue:在 TTL 内重复读取只触发一次仓库访问
  2. TestGetBooleanFallsBackToLastKnownValueOnRefreshError:刷新失败时回退到最近缓存值

测试正确使用 Invalidate 确保干净的初始状态,并通过 repo.reads 精确验证仓库访问次数。


11-34: stubRepo 实现完整且设计巧妙。

  • 支持按读取顺序返回不同值(values 切片)
  • 支持按读取顺序注入错误(errs 切片)
  • 正确实现 repository.SystemSettingRepository 接口的所有方法
internal/systemsettingcache/boolean.go (3)

26-49: 缓存逻辑设计合理,错误处理健壮。

GetBoolean 实现了三层读取策略:

  1. 优先返回 TTL 内的缓存值
  2. 缓存过期时从仓库刷新
  3. 仓库读取失败时回退到过期缓存值

这确保了在数据库瞬时故障期间,kill switch 不会意外关闭。


57-68: 缓存查询函数职责清晰。

  • getFreshBoolean:仅返回 TTL 内的新鲜值
  • getCachedBoolean:返回任意缓存值(用于错误回退)

这种分离使得 GetBoolean 的逻辑更清晰易读。

Also applies to: 70-78


86-88: 布尔值规范化与前端一致。

strings.EqualFold(strings.TrimSpace(value), "true") 与前端的 .trim().toLowerCase() === 'true' 功能等价,确保前后端对同一设置值的解释一致。

…-routes-page

# Conflicts:
#	internal/service/admin.go
@awsl233777
Copy link
Copy Markdown
Collaborator Author

Merged latest origin/main into this PR and resolved the conflict in internal/service/admin.go.

Conflict resolution kept the systemsettingcache invalidation import used by the proxy kill-switch settings path and did not reintroduce the now-unused usage import from the older branch side.

Validation run locally:

go test ./internal/service ./internal/handler ./internal/core

Result: passed.

@coderabbitai re-review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 31, 2026

@awsl233777 收到!冲突处理看起来很清晰——保留 systemsettingcache 失效路径的 import,丢弃已无用的 usage import,方向正确。本地回归验证通过也给了一层保障。即将触发完整 re-review。

(´。• ω •。`) 🐰

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/service/system_setting_validation_test.go`:
- Around line 92-95: 这些测试在修改 systemsettingcache.BooleanTTL 并写入全局缓存(键
"proxy_requests_disabled"),但只恢复了 TTL,没有清理缓存,导致跨测试污染。为每个相关测试(如
TestAdminServiceUpdateSettingInvalidatesProxyBooleanCache 及其他列出的用例)在
setup/teardown 或在 defer 中恢复 BooleanTTL 后也显式删除或清空该缓存键(例如调用相应的缓存清理方法,像
systemsettingcache.Delete("proxy_requests_disabled") 或
systemsettingcache.ClearKey("proxy_requests_disabled"),或使用 package 提供的全局 Clear
方法),确保每个测试结束时全局缓存回到干净状态以避免顺序依赖。

In `@web/src/components/settings/proxy-kill-switch-card.tsx`:
- Around line 12-18: The code currently treats a missing settings object as
"false" by computing proxyRequestsDisabled from settings even when settings is
undefined; change this so proxyRequestsDisabled is undefined (or a tri-state)
while settings are loading or failed, and ensure the UI uses isLoading or
proxyRequestsDisabled === undefined to render a loading/error state or disable
the toggle rather than showing "off"; specifically update the logic around
useSettings(), useUpdateSetting(), and the proxyRequestsDisabled calculation to
only derive a boolean when settings?.proxy_requests_disabled is present, and
guard the card render/path that writes settings so admins cannot flip the kill
switch until isLoading is false and settings is defined.
🪄 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: 5f1683f7-391c-4b16-9dc1-5c1dbe2f1770

📥 Commits

Reviewing files that changed from the base of the PR and between 8ecafbd and 745ded6.

📒 Files selected for processing (16)
  • cmd/maxx/main.go
  • internal/core/database.go
  • internal/domain/model.go
  • internal/handler/proxy.go
  • internal/handler/proxy_test.go
  • internal/service/admin.go
  • internal/service/system_setting_validation.go
  • internal/service/system_setting_validation_test.go
  • internal/systemsettingcache/boolean.go
  • internal/systemsettingcache/boolean_test.go
  • tests/e2e/proxy_setup_test.go
  • web/src/components/settings/proxy-kill-switch-card.tsx
  • web/src/locales/en.json
  • web/src/locales/zh.json
  • web/src/pages/client-routes/index.tsx
  • web/src/pages/settings/index.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: e2e
  • GitHub Check: playwright
🔇 Additional comments (14)
cmd/maxx/main.go (1)

466-466: LGTM!

internal/core/database.go (1)

455-455: LGTM!

tests/e2e/proxy_setup_test.go (1)

187-187: LGTM!

internal/handler/proxy.go (3)

20-22: LGTM!

Also applies to: 25-25, 39-39, 52-52, 59-59


113-118: LGTM!


320-326: LGTM!

internal/handler/proxy_test.go (3)

17-41: LGTM!


171-177: LGTM!

Also applies to: 179-197, 199-221


154-169: 确认 settingsTestRepo 类型已在包内定义

settingsTestRepo 并非未定义:internal/handler/admin_settings_test.go:13 已包含 type settingsTestRepo struct { ... },因此该测试用例无需改用 proxyBooleanSettingRepo

internal/systemsettingcache/boolean.go (1)

1-89: LGTM!

internal/systemsettingcache/boolean_test.go (1)

11-35: LGTM!

Also applies to: 36-54, 56-78

internal/domain/model.go (1)

691-691: LGTM!

internal/service/system_setting_validation.go (1)

15-32: LGTM!

internal/service/admin.go (1)

487-493: LGTM!

Also applies to: 509-515

Comment on lines +92 to +95
func TestAdminServiceUpdateSettingInvalidatesProxyBooleanCache(t *testing.T) {
oldTTL := systemsettingcache.BooleanTTL
systemsettingcache.BooleanTTL = time.Hour
defer func() { systemsettingcache.BooleanTTL = oldTTL }()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

补上全局缓存清理,避免测试之间相互污染。

这两条用例会修改 systemsettingcache.BooleanTTL 并填充全局缓存,但现在只恢复了 TTL,没有在结束时清掉 proxy_requests_disabled 的缓存值。后续同包测试如果也读取这个 key,结果会依赖执行顺序。

🧪 建议修复
 func TestAdminServiceUpdateSettingInvalidatesProxyBooleanCache(t *testing.T) {
 	oldTTL := systemsettingcache.BooleanTTL
 	systemsettingcache.BooleanTTL = time.Hour
-	defer func() { systemsettingcache.BooleanTTL = oldTTL }()
+	t.Cleanup(func() {
+		systemsettingcache.BooleanTTL = oldTTL
+		systemsettingcache.Invalidate(domain.SettingKeyProxyRequestsDisabled)
+	})
@@
 func TestAdminServiceDeleteSettingInvalidatesProxyBooleanCache(t *testing.T) {
 	oldTTL := systemsettingcache.BooleanTTL
 	systemsettingcache.BooleanTTL = time.Hour
-	defer func() { systemsettingcache.BooleanTTL = oldTTL }()
+	t.Cleanup(func() {
+		systemsettingcache.BooleanTTL = oldTTL
+		systemsettingcache.Invalidate(domain.SettingKeyProxyRequestsDisabled)
+	})

Also applies to: 100-110, 113-116, 121-131

🤖 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/service/system_setting_validation_test.go` around lines 92 - 95,
这些测试在修改 systemsettingcache.BooleanTTL 并写入全局缓存(键 "proxy_requests_disabled"),但只恢复了
TTL,没有清理缓存,导致跨测试污染。为每个相关测试(如
TestAdminServiceUpdateSettingInvalidatesProxyBooleanCache 及其他列出的用例)在
setup/teardown 或在 defer 中恢复 BooleanTTL 后也显式删除或清空该缓存键(例如调用相应的缓存清理方法,像
systemsettingcache.Delete("proxy_requests_disabled") 或
systemsettingcache.ClearKey("proxy_requests_disabled"),或使用 package 提供的全局 Clear
方法),确保每个测试结束时全局缓存回到干净状态以避免顺序依赖。

Comment on lines +12 to +18
const { data: settings, isLoading } = useSettings();
const updateSetting = useUpdateSetting();
const { t } = useTranslation();

const proxyRequestsDisabled =
(settings?.proxy_requests_disabled ?? '').trim().toLowerCase() === 'true';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

不要把“配置未加载成功”默认为关闭状态。

这里 settings 缺失时会把 kill switch 解释成 false,而且 isLoading 结束后就直接渲染卡片。若读取设置失败,管理员会看到“已关闭”并可能误操作覆盖真实值。

🔧 建议修复
 export function ProxyKillSwitchCard({ className }: ProxyKillSwitchCardProps) {
-  const { data: settings, isLoading } = useSettings();
+  const { data: settings, isLoading } = useSettings();
   const updateSetting = useUpdateSetting();
   const { t } = useTranslation();
 
+  if (isLoading || settings === undefined) return null;
+
   const proxyRequestsDisabled =
-    (settings?.proxy_requests_disabled ?? '').trim().toLowerCase() === 'true';
+    (settings.proxy_requests_disabled ?? '').trim().toLowerCase() === 'true';
@@
-  if (isLoading) return null;
-
   return (

Also applies to: 26-26

🤖 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/settings/proxy-kill-switch-card.tsx` around lines 12 - 18,
The code currently treats a missing settings object as "false" by computing
proxyRequestsDisabled from settings even when settings is undefined; change this
so proxyRequestsDisabled is undefined (or a tri-state) while settings are
loading or failed, and ensure the UI uses isLoading or proxyRequestsDisabled ===
undefined to render a loading/error state or disable the toggle rather than
showing "off"; specifically update the logic around useSettings(),
useUpdateSetting(), and the proxyRequestsDisabled calculation to only derive a
boolean when settings?.proxy_requests_disabled is present, and guard the card
render/path that writes settings so admins cannot flip the kill switch until
isLoading is false and settings is defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants