Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough为 API 令牌引入 DevMode 布尔标志并将其贯穿域模型、执行器状态、数据库模型、HTTP 处理器、备份/恢复与前端，同时基于该标志调整请求详情清理与数据库清理条件。 Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "UI (Admin/Client)"
participant Handler as "Proxy Handler"
participant Flow as "Flow Context"
participant Executor as "Executor / Middleware"
participant DB as "SQLite DB"
UI->>Handler: 请求或切换令牌 DevMode
Handler->>Flow: c.Set(KeyAPITokenDevMode, apiToken.DevMode)
Flow->>Executor: 启动中间件/执行（包含 state.apiTokenDevMode）
Executor->>Executor: shouldClearRequestDetailFor(state) -> clearDetail
Executor->>DB: ClearDetailOlderThan(...) WHERE dev_mode = 0
DB-->>Executor: 更新结果
Executor-->>UI: 返回（包含或省略 request/response 详情 取决于 clearDetail）
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/executor/middleware_dispatch.go (2)
310-329:⚠️ Potential issue | 🟠 Major错误路径遗漏了
clearDetail清理逻辑。当
clearDetail为 true 时，此上下文取消路径在调用proxyRequestRepo.Update前未清理RequestInfo/ResponseInfo。RequestInfo可能在 ingress 中已设置，会被意外持久化。类似问题也存在于 Line 357-378 的重试等待取消路径。
🐛 建议修复：在更新前添加清理逻辑
if ok && ctx.Err() != nil { proxyReq.Status = "CANCELLED" proxyReq.EndTime = time.Now() proxyReq.Duration = proxyReq.EndTime.Sub(proxyReq.StartTime) if ctx.Err() == context.Canceled { proxyReq.Error = "client disconnected" } else if ctx.Err() == context.DeadlineExceeded { proxyReq.Error = "request timeout" } else { proxyReq.Error = ctx.Err().Error() } + if clearDetail { + proxyReq.RequestInfo = nil + proxyReq.ResponseInfo = nil + } _ = e.proxyRequestRepo.Update(proxyReq)同样需要在 Line 369 的
proxyRequestRepo.Update前添加相同的清理逻辑。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/executor/middleware_dispatch.go` around lines 310 - 329, The cancellation/error handling paths that call e.proxyRequestRepo.Update (inside the block handling proxyErr and the retry-wait cancellation block) must respect the proxyReq.ClearDetail flag and remove/zero-out RequestInfo and ResponseInfo before persisting; update the branches in middleware_dispatch.go where proxyErr is handled (the block that sets proxyReq.Status = "CANCELLED", sets Error and Duration) and the retry-wait cancel path (the later block that also calls proxyRequestRepo.Update) to check proxyReq.ClearDetail and clear the RequestInfo/ResponseInfo fields prior to calling e.proxyRequestRepo.Update and before e.broadcaster.BroadcastProxyRequest so no sensitive request/response details get persisted or broadcast. Ensure you modify both locations that call proxyRequestRepo.Update so behavior is consistent.
284-305:⚠️ Potential issue | 🟠 Major错误路径的
RequestInfo未被清理。在 Line 286-292，
ResponseInfo正确地根据!clearDetail条件设置。但在 Line 305 调用proxyRequestRepo.Update前，proxyReq.RequestInfo未被清理。与成功路径（Lines 224-227）的处理不一致。
🐛 建议修复
proxyReq.Cost = attemptRecord.Cost proxyReq.TTFT = attemptRecord.TTFT +if clearDetail { + proxyReq.RequestInfo = nil +} _ = e.proxyRequestRepo.Update(proxyReq)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/executor/middleware_dispatch.go` around lines 284 - 305, The error path fails to clear sensitive request details before persisting: ensure that when clearDetail is true you set proxyReq.RequestInfo = nil (or an empty RequestInfo) the same way ResponseInfo is conditionally omitted; update the block around responseCapture handling (where proxyReq.ResponseInfo is set based on clearDetail and before calling e.proxyRequestRepo.Update(proxyReq)) to explicitly clear proxyReq.RequestInfo whenever clearDetail is true so the error-path persistence matches the success-path behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/executor/middleware_dispatch.go`:
- Around line 310-329: The cancellation/error handling paths that call
e.proxyRequestRepo.Update (inside the block handling proxyErr and the retry-wait
cancellation block) must respect the proxyReq.ClearDetail flag and
remove/zero-out RequestInfo and ResponseInfo before persisting; update the
branches in middleware_dispatch.go where proxyErr is handled (the block that
sets proxyReq.Status = "CANCELLED", sets Error and Duration) and the retry-wait
cancel path (the later block that also calls proxyRequestRepo.Update) to check
proxyReq.ClearDetail and clear the RequestInfo/ResponseInfo fields prior to
calling e.proxyRequestRepo.Update and before e.broadcaster.BroadcastProxyRequest
so no sensitive request/response details get persisted or broadcast. Ensure you
modify both locations that call proxyRequestRepo.Update so behavior is
consistent.
- Around line 284-305: The error path fails to clear sensitive request details
before persisting: ensure that when clearDetail is true you set
proxyReq.RequestInfo = nil (or an empty RequestInfo) the same way ResponseInfo
is conditionally omitted; update the block around responseCapture handling
(where proxyReq.ResponseInfo is set based on clearDetail and before calling
e.proxyRequestRepo.Update(proxyReq)) to explicitly clear proxyReq.RequestInfo
whenever clearDetail is true so the error-path persistence matches the
success-path behavior.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
internal/domain/backup.gointernal/domain/model.gointernal/executor/executor.gointernal/executor/flow_state.gointernal/executor/middleware_dispatch.gointernal/executor/middleware_ingress.gointernal/flow/keys.gointernal/handler/admin.gointernal/handler/proxy.gointernal/repository/sqlite/api_token.gointernal/repository/sqlite/models.gointernal/repository/sqlite/proxy_request.gointernal/repository/sqlite/proxy_upstream_attempt.gointernal/service/backup.goweb/src/lib/transport/types.tsweb/src/locales/en.jsonweb/src/locales/zh.jsonweb/src/pages/api-tokens/index.tsx
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (5)
internal/repository/sqlite/proxy_upstream_attempt.go (1)
internal/repository/sqlite/models.go (4)
ProxyRequest(198-232)ProxyRequest(234-234)ProxyUpstreamAttempt(237-262)ProxyUpstreamAttempt(264-264)
internal/executor/middleware_ingress.go (1)
internal/flow/keys.go (1)
KeyAPITokenDevMode(21-21)
internal/handler/proxy.go (1)
internal/flow/keys.go (1)
KeyAPITokenDevMode(21-21)
internal/executor/executor.go (6)
internal/domain/adapter_event.go (2)
AdapterEventChan(40-40)EventResponseInfo(10-10)internal/domain/model.go (3)
ProxyUpstreamAttempt(361-415)RequestInfo(274-279)ResponseInfo(280-284)internal/repository/sqlite/models.go (2)
ProxyUpstreamAttempt(237-262)ProxyUpstreamAttempt(264-264)web/src/lib/transport/types.ts (3)
ProxyUpstreamAttempt(245-274)RequestInfo(175-180)ResponseInfo(182-186)web/src/lib/transport/index.ts (3)
ProxyUpstreamAttempt(28-28)RequestInfo(30-30)ResponseInfo(31-31)internal/adapter/client/adapter.go (1)
RequestInfo(17-20)
web/src/pages/api-tokens/index.tsx (6)
internal/domain/model.go (1)
APIToken(707-742)internal/repository/sqlite/models.go (2)
APIToken(134-146)APIToken(148-148)web/src/lib/transport/types.ts (1)
APIToken(609-623)web/src/lib/transport/index.ts (1)
APIToken(70-70)web/src/components/ui/index.ts (4)
TableHead(13-13)TableCell(15-15)Switch(51-51)Badge(20-20)web/src/components/ui/badge.tsx (1)
Badge(54-54)
🔇 Additional comments (26)
internal/domain/backup.go (1)
88-88: 字段扩展方向正确，备份模型已覆盖 DevMode。该改动与备份/恢复链路的功能目标一致，数据模型扩展清晰。
internal/flow/keys.go (1)
21-21: 新增 Flow Key 合理，便于在链路中传递令牌开发者模式状态。web/src/lib/transport/types.ts (1)
619-619: 类型层同步到位：运行态必填、备份态可选的设计是合理的。Also applies to: 808-808
web/src/locales/en.json (1)
810-812: 国际化键补充完整，可支撑 DevMode 开关与状态展示。web/src/pages/api-tokens/index.tsx (1)
142-147: DevMode 前端功能闭环完整（操作、展示、状态文案）且实现清晰。Also applies to: 271-271, 335-355
internal/domain/model.go (1)
357-358: 领域模型扩展正确，DevMode 在请求与令牌两个核心实体上都已覆盖。Also applies to: 728-729
internal/service/backup.go (1)
197-197: 备份导出/导入链路对 DevMode 的映射是成对且一致的。Also applies to: 743-743
internal/executor/flow_state.go (1)
25-25: 执行态新增apiTokenDevMode字段合理，便于后续链路消费该标志位。web/src/locales/zh.json (1)
810-812: LGTM!本地化键值添加正确，与现有结构保持一致。
devMode和devModeEnabled使用相同的文本是合理的，分别用于列标题和状态显示。internal/handler/admin.go (1)
1067-1097: LGTM!DevMode 字段的处理遵循了现有的部分更新模式，使用
*bool指针正确区分了"未提供"和"显式设置为 false"的情况。实现与IsEnabled等其他字段保持一致。internal/handler/proxy.go (1)
144-148: LGTM!DevMode 在 token 认证成功后正确存储到 flow context 中，与
KeyAPITokenID的处理模式一致。需注意：当
apiToken为 nil 时（未启用 token 认证或未提供 token），KeyAPITokenDevMode不会被设置。从internal/executor/middleware_ingress.go的代码来看，下游会正确处理这种情况（获取失败时state.apiTokenDevMode保持零值false），符合预期行为。internal/repository/sqlite/proxy_upstream_attempt.go (1)
244-258: LGTM!使用子查询过滤
dev_mode = 0的请求是正确的实现方式，因为ProxyUpstreamAttempt本身没有dev_mode字段，需要关联ProxyRequest表。逻辑与proxy_request.go中的处理保持一致。internal/repository/sqlite/proxy_request.go (3)
428-429: LGTM!在清理条件中添加
dev_mode = 0确保了开发者模式的请求详情被永久保留，符合 PR 需求。
477-477: LGTM!DevMode 的 boolean-to-int 映射与
IsStream等其他字段的处理模式一致。
517-517: LGTM!从数据库读取时正确将整数转换回布尔值。
internal/executor/middleware_ingress.go (2)
54-58: LGTM!从 flow context 读取
KeyAPITokenDevMode的模式与其他字段（如KeyClientType、KeyProjectID）保持一致，类型断言能正确处理键不存在或类型不匹配的情况。
94-117: LGTM!
DevMode正确传递到ProxyRequest对象用于持久化。- 使用
shouldClearRequestDetailFor(state)替换原有的直接调用，使详情清理逻辑能够感知 DevMode 状态。!clearDetail的条件判断语义正确：当不应清理详情时（包括开发者模式场景），才填充请求信息。internal/repository/sqlite/models.go (2)
142-142: LGTM!
DevMode字段使用int类型存储布尔值，与IsEnabled等字段保持一致。default:0确保现有记录和未显式设置的新记录默认为普通模式。
231-231: LGTM!
ProxyRequest的DevMode字段定义与APIToken保持一致，便于数据清理逻辑根据此字段判断是否保留请求详情。internal/repository/sqlite/api_token.go (2)
36-44: LGTM!
Update方法正确包含dev_mode字段，确保通过 API 更新 token 时 DevMode 状态能够持久化。
103-143: LGTM!
toModel和toDomain方法的 DevMode 转换实现正确，与IsEnabled等字段的处理模式完全一致。internal/executor/executor.go (2)
415-421: 实现清晰正确。新增的
shouldClearRequestDetailFor辅助函数逻辑清晰：
- 优先检查令牌级别的开发者模式（
apiTokenDevMode）- 若开发者模式开启则保留详情（返回
false）- 否则回退到全局配置
这与 PR 目标"开启开发者模式的令牌，其请求详情将被永久保留"一致。
317-322: 事件过滤逻辑正确。函数签名扩展和事件处理逻辑良好：
clearDetail参数控制是否在实时广播中保留 RequestInfo/ResponseInfo- 即使
clearDetail为 true，仍正确处理 Metrics、ResponseModel、FirstToken 等事件- 这确保了即使不保留详情，性能指标仍能正常采集
Also applies to: 360-368
internal/executor/middleware_dispatch.go (3)
28-28:clearDetail初始化时机和位置正确。在 dispatch 开始时计算一次
clearDetail，避免在循环中重复调用，性能良好。
183-186: 成功路径的清理逻辑完整正确。成功路径的实现覆盖全面：
- Line 183-186：清理
attemptRecord的详情- Line 204-210：仅在
!clearDetail时设置ResponseInfo- Line 224-227：最终清理确保
RequestInfo（来自 ingress）也被清除Also applies to: 204-210, 224-227
388-392: 全部路由失败路径的清理逻辑正确。
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/executor/middleware_dispatch.go (1)
327-330: 可选重构：考虑提取重复的清理逻辑。
if clearDetail { proxyReq.RequestInfo = nil; proxyReq.ResponseInfo = nil }模式在文件中重复了 6 次。可以考虑提取为辅助方法以减少重复：♻️ 可选：提取清理辅助方法
在
Executor或相关结构体上添加辅助方法：func clearProxyRequestDetail(req *domain.ProxyRequest, clearDetail bool) { if clearDetail { req.RequestInfo = nil req.ResponseInfo = nil } }然后在各处调用：
- if clearDetail { - proxyReq.RequestInfo = nil - proxyReq.ResponseInfo = nil - } + clearProxyRequestDetail(proxyReq, clearDetail)当前内联写法也可接受，因为它使每个退出点的行为更加明确。
Also applies to: 378-381, 401-404
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/executor/middleware_dispatch.go` around lines 327 - 330, The repeated pattern clearing proxy request details (if clearDetail { proxyReq.RequestInfo = nil; proxyReq.ResponseInfo = nil }) appears multiple times (e.g., near the exit paths that reference proxyReq and clearDetail); extract this into a small helper such as clearProxyRequestDetail(req *domain.ProxyRequest, clearDetail bool) and replace the inline blocks with calls to that helper at each exit point (locations currently using proxyReq and clearDetail around the middleware dispatch flows). Ensure the helper sets RequestInfo and ResponseInfo to nil when clearDetail is true so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/executor/middleware_dispatch.go`:
- Around line 327-330: The repeated pattern clearing proxy request details (if
clearDetail { proxyReq.RequestInfo = nil; proxyReq.ResponseInfo = nil }) appears
multiple times (e.g., near the exit paths that reference proxyReq and
clearDetail); extract this into a small helper such as
clearProxyRequestDetail(req *domain.ProxyRequest, clearDetail bool) and replace
the inline blocks with calls to that helper at each exit point (locations
currently using proxyReq and clearDetail around the middleware dispatch flows).
Ensure the helper sets RequestInfo and ResponseInfo to nil when clearDetail is
true so behavior remains identical.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/executor/middleware_dispatch.go
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (1)
internal/executor/middleware_dispatch.go (2)
internal/domain/model.go (2)
RequestInfo(274-279)ResponseInfo(280-284)web/src/lib/transport/types.ts (2)
RequestInfo(175-180)ResponseInfo(182-186)
🔇 Additional comments (4)
internal/executor/middleware_dispatch.go (4)
131-131: LGTM!将
clearDetail传递给异步事件处理器，确保流式事件处理与同步路径保持一致的详情清理行为。
183-227: 成功路径的详情清理逻辑正确。采用了防御性编程模式：先条件性设置
ResponseInfo（第 204-210 行），再在最终更新前统一清理（第 224-227 行）。这确保了即使RequestInfo在其他地方（如 ingress 中间件）被设置，也能在clearDetail=true时被正确清除。
269-308: 错误路径的详情处理与成功路径保持一致。失败场景下的
attemptRecord和proxyReq详情清理逻辑正确，确保开发者模式标志在所有结果状态下都被尊重。
28-28: 集中化详情清理决策是优秀的设计模式。在调度开始时一次计算
clearDetail标志，随后在整个请求处理流程的多个关键节点（如事件处理、响应映射、异常处理等）中一致性地使用，确保了行为的统一性和可维护性。已验证：
shouldClearRequestDetailFor在internal/executor/executor.go中正确实现，当apiTokenDevMode为true时返回false，准确反映了"开发者模式开启 → 保留详情"的设计意图。此外，该函数在调用时也正确地作为备选方案回退到全局配置shouldClearRequestDetail()。
2 participants
变更
验证
Summary by CodeRabbit
新功能
本地化
管理