fix: restore tenant-scoped oauth broadcasts and align websocket tests#442
fix: restore tenant-scoped oauth broadcasts and align websocket tests#442awsl233777 wants to merge 7 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough在 OAuth 会话中加入 TenantID 并将 CreateSession 签名改为接收 tenantID；CompleteSession 在删除会话前读取 TenantID 并在 TenantID != 0 时通过租户范围广播（BroadcastMessageToTenant），其余为全局广播。为此扩展了广播接口、Wails 实现、WebSocket Hub、仓储接口及相关测试与处理路径。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OAuthManager
participant SessionStore as Repository
participant Broadcaster
participant Wails as WailsBroadcaster
Client->>OAuthManager: CreateSession(state, tenantID)
OAuthManager->>SessionStore: persist OAuthSession{State, TenantID, ...}
OAuthManager-->>Client: return session + PKCE
Client->>OAuthManager: CompleteSession(state, result)
OAuthManager->>SessionStore: lookup session by state -> returns TenantID
OAuthManager->>SessionStore: delete session
alt TenantID != 0
OAuthManager->>Broadcaster: BroadcastMessageToTenant(tenantID, type, data)
else TenantID == 0
OAuthManager->>Broadcaster: BroadcastMessage(type, data)
end
Broadcaster->>Wails: emitWailsEvent(type, data)
Wails->>Client: deliver websocket/event update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
诗文
🚥 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 docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
internal/adapter/provider/codex/service.go (1)
86-93: 建议：Claude 和 Codex 的 OAuth 管理器存在明显的代码重复。
OAuthSession结构体、CreateSession、GetSession和CompleteSession方法在两个服务中几乎完全相同。可以考虑将通用的 OAuth 会话管理逻辑抽取到一个共享的基础包中，仅保留特定于提供商的部分（如事件类型名称"claude_oauth_result"vs"codex_oauth_result"）。此重构不阻塞当前 PR，可作为后续改进。
Also applies to: 140-158, 181-204
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/adapter/provider/codex/service.go` around lines 86 - 93, The OAuthSession struct and the methods CreateSession, GetSession, and CompleteSession are duplicated between the Claude and Codex services; extract the shared OAuth session management into a common package (e.g., internal/oauth or internal/adapter/oauth) containing the OAuthSession type and generic CreateSession/GetSession/CompleteSession implementations, and have the provider-specific services call into that package, keeping only provider-specific differences (such as the event type string "claude_oauth_result" vs "codex_oauth_result") in their own code; update references in the Codex service (OAuthSession, CreateSession, GetSession, CompleteSession) to use the new shared package API.
🤖 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/adapter/provider/claude/service.go`:
- Line 117: CreateSession's signature now requires tenantID uint64 but callers
still use the old signature; update each call site (notably the invocations in
internal/handler/claude.go and internal/handler/codex.go) to fetch the tenant ID
from context like in internal/handler/antigravity.go: use
maxxctx.GetTenantID(ctx), default to domain.DefaultTenantID when zero, then pass
that tenantID into OAuthManager.CreateSession(state, tenantID). Ensure you
import or reference maxxctx/domain as needed and propagate any context variable
(ctx) already available at the call site.
- Around line 169-173: 接口 event.Broadcaster 缺少 BroadcastMessageToTenant 方法，且
OAuthManager.CreateSession 签名已变更导致调用不匹配；为修复：在 internal/event/broadcaster.go 的
event.Broadcaster 接口中添加 BroadcastMessageToTenant(tenantID uint64, event string,
payload interface{}) 方法签名（与现有 BroadcastMessage 对称），并在
internal/adapter/provider/claude/service.go 和
internal/adapter/provider/codex/service.go 保留对 BroadcastMessageToTenant
的调用；然后更新调用 CreateSession 的处理器 internal/handler/claude.go 和
internal/handler/codex.go，将缺失的 tenantID 参数一并传入（参考
internal/handler/antigravity.go 的传参方式和获取 tenantID 的逻辑）。
In `@internal/adapter/provider/codex/service.go`:
- Around line 198-202: The build fails because service.go calls
BroadcastMessageToTenant on m.broadcaster but event.Broadcaster lacks that
method; add a BroadcastMessageToTenant(ctx context.Context, tenantID int64,
topic string, payload interface{}) (or matching signature used in other
services) to the event.Broadcaster interface and implement it in the concrete
broadcaster type(s) (the same change applied for Claude in
internal/adapter/provider/claude/service.go should be mirrored here); update any
call sites (m.broadcaster.BroadcastMessageToTenant and tests) to use the new
signature so the call in internal/adapter/provider/codex/service.go compiles.
- Line 141: internal/handler/codex.go still calls
h.oauthManager.CreateSession(state) but the signature changed to
CreateSession(state string, tenantID uint64) returning (*OAuthSession,
*PKCEChallenge, error); update that call to obtain tenantID via
maxxctx.GetTenantID(ctx) and default to domain.DefaultTenantID when zero, then
call h.oauthManager.CreateSession(state, tenantID) and capture the returned
values (e.g. _, pkce, err) following the pattern used in antigravity.go.
In `@internal/repository/interfaces.go`:
- Around line 160-165: The SQLite implementation is missing the
ProxyUpstreamAttemptRepository methods CountByTenant and
StreamForCostCalcByTenant; implement CountByTenant to run a COUNT query against
the attempts table filtered by tenant_id (respect TenantIDAll semantics used
elsewhere) and return (int64, error), and implement StreamForCostCalcByTenant to
mirror the existing StreamForCostCalc logic but add tenant_id filtering: fetch
rows in batches (use LIMIT/OFFSET or keyset pagination consistent with other
repository methods), map each row to domain.AttemptCostData, invoke the provided
callback with the batch, and stop/return immediately if the callback returns an
error; ensure method signatures exactly match CountByTenant and
StreamForCostCalcByTenant on ProxyUpstreamAttemptRepository and reuse existing
DB helpers and error handling patterns used in proxy_upstream_attempt.go.
---
Nitpick comments:
In `@internal/adapter/provider/codex/service.go`:
- Around line 86-93: The OAuthSession struct and the methods CreateSession,
GetSession, and CompleteSession are duplicated between the Claude and Codex
services; extract the shared OAuth session management into a common package
(e.g., internal/oauth or internal/adapter/oauth) containing the OAuthSession
type and generic CreateSession/GetSession/CompleteSession implementations, and
have the provider-specific services call into that package, keeping only
provider-specific differences (such as the event type string
"claude_oauth_result" vs "codex_oauth_result") in their own code; update
references in the Codex service (OAuthSession, CreateSession, GetSession,
CompleteSession) to use the new shared package API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dd56afc0-17de-48d9-8895-c28c6deeda3e
📒 Files selected for processing (4)
internal/adapter/provider/claude/service.gointernal/adapter/provider/codex/service.gointernal/handler/websocket_test.gointernal/repository/interfaces.go
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🪛 GitHub Actions: E2E Playwright Tests
internal/adapter/provider/claude/service.go
[error] 170-170: go build failed: m.broadcaster.BroadcastMessageToTenant undefined (type event.Broadcaster has no field or method BroadcastMessageToTenant)
🪛 GitHub Actions: E2E Tests
internal/adapter/provider/claude/service.go
[error] 170-170: go test failed: event.Broadcaster has no field or method BroadcastMessageToTenant (m.broadcaster.BroadcastMessageToTenant undefined).
🪛 GitHub Actions: PR Checks
internal/adapter/provider/claude/service.go
[error] 170-170: go vet failed: m.broadcaster.BroadcastMessageToTenant undefined (type event.Broadcaster has no field or method BroadcastMessageToTenant).
🪛 GitHub Check: Backend Checks
internal/adapter/provider/claude/service.go
[failure] 170-170:
m.broadcaster.BroadcastMessageToTenant undefined (type event.Broadcaster has no field or method BroadcastMessageToTenant)
internal/adapter/provider/codex/service.go
[failure] 199-199:
m.broadcaster.BroadcastMessageToTenant undefined (type event.Broadcaster has no field or method BroadcastMessageToTenant)
🪛 GitHub Check: e2e
internal/adapter/provider/claude/service.go
[failure] 170-170:
m.broadcaster.BroadcastMessageToTenant undefined (type event.Broadcaster has no field or method BroadcastMessageToTenant)
internal/adapter/provider/codex/service.go
[failure] 199-199:
m.broadcaster.BroadcastMessageToTenant undefined (type event.Broadcaster has no field or method BroadcastMessageToTenant)
🪛 GitHub Check: playwright
internal/adapter/provider/claude/service.go
[failure] 170-170:
m.broadcaster.BroadcastMessageToTenant undefined (type event.Broadcaster has no field or method BroadcastMessageToTenant)
internal/adapter/provider/codex/service.go
[failure] 199-199:
m.broadcaster.BroadcastMessageToTenant undefined (type event.Broadcaster has no field or method BroadcastMessageToTenant)
🔇 Additional comments (3)
internal/handler/websocket_test.go (3)
13-50: 测试更新正确对齐了 hub 的内部消息类型。测试代码正确地将
broadcast通道类型从WSMessage更新为queuedWSMessage，并通过queued.message解包访问底层消息。快照隔离验证逻辑保持完整。
52-108: 其余测试函数的更新保持一致性。
BroadcastProxyUpstreamAttempt和BroadcastDrop测试同样正确地适配了新的消息封装结构。
110-185: 死锁测试和 BroadcastMessage 测试更新完整。通道填充和消息断言都正确使用了
queuedWSMessage类型。
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/event/wails_broadcaster_desktop.go`:
- Around line 76-82: The WebSocketHub currently lacks an implementation of
BroadcastMessageToTenant which causes a runtime panic when
WailsBroadcaster.BroadcastMessageToTenant calls
w.inner.BroadcastMessageToTenant; implement BroadcastMessageToTenant on the
WebSocketHub type (matching the Broadcaster signature) and delegate to its
existing broadcast mechanisms so it supports tenant-scoped messages;
additionally, modify WailsBroadcaster.emitWailsEvent call from
BroadcastMessageToTenant (or the data passed into it) to include tenantID in the
payload so desktop clients receive tenant context, or if the desktop app is
truly single-tenant, add a clear comment on
WailsBroadcaster.BroadcastMessageToTenant stating tenantID is ignored and why.
In `@internal/event/wails_broadcaster_http.go`:
- Around line 55-60: The WebSocketHub type lacks the
BroadcastMessageToTenant(tenantID uint64, messageType string, data interface{})
method required by the Broadcaster interface; add this method to WebSocketHub
(same signature as WailsBroadcaster.BroadcastMessageToTenant) and implement it
to send the message only to connections belonging to the given tenantID (e.g.,
mirror the logic used in existing broadcast methods but filter by tenant),
ensuring thread-safety (use the hub's mutex or dispatch channel) and making it a
no-op when no tenant connections exist to avoid runtime panic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ec3a4a38-6ccc-48cf-8fd9-85237b1d328f
📒 Files selected for processing (3)
internal/event/broadcaster.gointernal/event/wails_broadcaster_desktop.gointernal/event/wails_broadcaster_http.go
✅ Files skipped from review due to trivial changes (1)
- internal/event/broadcaster.go
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
|
@coderabbitai re-review ！Stop making breaking changes, do a proper review！ |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll do a thorough audit of the breaking changes in this PR right away. 🏁 Script executed: Length of output: 80004
1.
|
| Caller | Location |
|---|---|
h.oauthManager.CreateSession(state, tenantID) |
internal/handler/claude.go:111 |
h.oauthManager.CreateSession(state, tenantID) |
internal/handler/codex.go:157 |
h.oauthManager.CreateSession(state, tenantID) |
internal/handler/antigravity.go:186 |
No unpatched callers exist in the codebase.
2. ProxyUpstreamAttemptRepository interface additions — ✅ Fully implemented
Both new methods (CountByTenant, StreamForCostCalcByTenant) are already implemented in the only concrete struct (internal/repository/sqlite/ProxyUpstreamAttemptRepository). Critically, the existing methods delegate correctly to preserve backward compatibility:
CountAll()→CountByTenant(domain.TenantIDAll)✅StreamForCostCalc()→StreamForCostCalcByTenant(domain.TenantIDAll, ...)✅
E2E tests use the real sqlite implementation, so no mocks need updating.
3. Broadcaster interface addition (BroadcastMessageToTenant) — ✅ All implementors updated
| Implementation | Status |
|---|---|
NopBroadcaster |
Implemented (no-op) ✅ |
WailsBroadcaster (HTTP build) |
Delegates to inner only — correct, no Wails events in HTTP mode ✅ |
WailsBroadcaster (Desktop build) |
Delegates to inner + calls emitWailsEvent — consistent with BroadcastMessage desktop behavior ✅ |
WebSocketHub |
Delegates to broadcastJSONMessage with scoped=true ✅ |
No other Broadcaster implementations exist in the codebase.
4. CompleteSession sequencing (Claude & Codex) — ✅ Correct
The session's TenantID is captured before the session is deleted, preventing tenant scope loss:
lookup session → capture tenantID → delete session → broadcast with tenantID
5. WebSocket test alignment — ✅ Correct
queuedWSMessage is the actual type of WebSocketHub.broadcast (chan queuedWSMessage). All five test functions are correctly updated to construct with make(chan queuedWSMessage, ...) and extract queued.message before asserting on Type and Data.
Summary: This PR is internally consistent and complete. All interface additions are fully implemented, all call-site signatures match, and there are no unpatched callers, missing implementations, or behavioral regressions. The changes correctly restore the tenant-scoped OAuth broadcasting and repository interface that were inadvertently removed in a prior refactor.
🧠 Learnings used
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.
|
@coderabbitai 你怎么一直等待中 认真点 |
|
Tip For best results, initiate chat on the files or code changes.
综合结论：此 PR 内部一致、完整，无破坏性变更或回归风险。 如有其他具体疑虑，请告知！ 🧠 Learnings used |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/repository/sqlite/proxy_upstream_attempt.go (1)
93-98: 建议复用tenantScope辅助函数以保持租户过滤逻辑一致。
CountByTenant()已经使用tenantScope()来处理租户过滤，但StreamForCostCalcByTenant()在第 93-98 行手动重新实现了相同的逻辑。这会导致维护问题：当tenantScope扩展以支持共享租户、NULL 处理或额外的谓词时，流式扫描不会自动更新，导致计数和实际处理的数据集出现差异。建议改为：
♻️ 建议改法
- query := r.db.gorm.Table("proxy_upstream_attempts"). - Select("id, proxy_request_id, response_model, mapped_model, request_model, input_token_count, output_token_count, cache_read_count, cache_write_count, cache_5m_write_count, cache_1h_write_count, cost"). - Where("id > ?", lastID) - if tenantID != domain.TenantIDAll { - query = query.Where("tenant_id = ?", tenantID) - } + query := tenantScope( + r.db.gorm.Table("proxy_upstream_attempts"). + Select("id, proxy_request_id, response_model, mapped_model, request_model, input_token_count, output_token_count, cache_read_count, cache_write_count, cache_5m_write_count, cache_1h_write_count, cost"). + Where("id > ?", lastID), + tenantID, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/repository/sqlite/proxy_upstream_attempt.go` around lines 93 - 98, StreamForCostCalcByTenant currently reimplements tenant filtering (the query built on r.db.gorm.Table("proxy_upstream_attempts") with Where("tenant_id = ?")), which duplicates CountByTenant's use of tenantScope; change StreamForCostCalcByTenant to apply the existing tenantScope helper instead of the manual Where so the same tenant predicates (including shared/NULL handling) are reused—locate StreamForCostCalcByTenant and replace the conditional tenant Where with a call to tenantScope on the GORM query (same pattern used in CountByTenant) so future tenant logic changes automatically apply to both counting and streaming.
🤖 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/codex.go`:
- Around line 210-211: The /api/codex/oauth/start handler is missing tenant
validation: call to maxxctx.GetTenantID(r.Context()) in codex.go followed by
h.StartOAuth(tenantID) proceeds even when tenantID == 0 which allows session
creation to fall back to global broadcasts (see CompleteSession,
BroadcastMessage vs BroadcastMessageToTenant); add an explicit check in the
handler after GetTenantID to reject tenantID == 0 by returning an HTTP error
(unauthorized/bad request) and avoid calling StartOAuth, ensuring multi-tenant
isolation is preserved.
In `@internal/handler/websocket.go`:
- Around line 286-289: The current BroadcastLog implementation incorrectly
hardcodes domain.DefaultTenantID and forces default-only visibility; revert
BroadcastLog to its original non-tenantized behavior (do not force default
tenant) by calling h.broadcastJSONMessage with the tenant parameter that
represents instance-wide delivery (e.g. empty string or the previous sentinel
used for instance-wide logs) and keep the existing flags, and instead add a new
explicit tenant-scoped API such as BroadcastLogForTenant(tenantID string,
message string) or BroadcastLogDefaultTenant(message string) that calls
h.broadcastJSONMessage(domain.DefaultTenantID, ...), so callers in
internal/event/* can continue working without breaking behavior while consumers
can opt into tenant-scoped logs.
---
Nitpick comments:
In `@internal/repository/sqlite/proxy_upstream_attempt.go`:
- Around line 93-98: StreamForCostCalcByTenant currently reimplements tenant
filtering (the query built on r.db.gorm.Table("proxy_upstream_attempts") with
Where("tenant_id = ?")), which duplicates CountByTenant's use of tenantScope;
change StreamForCostCalcByTenant to apply the existing tenantScope helper
instead of the manual Where so the same tenant predicates (including shared/NULL
handling) are reused—locate StreamForCostCalcByTenant and replace the
conditional tenant Where with a call to tenantScope on the GORM query (same
pattern used in CountByTenant) so future tenant logic changes automatically
apply to both counting and streaming.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e0f9e073-7186-48a5-a454-b1b0a7918318
📒 Files selected for processing (5)
internal/handler/claude.gointernal/handler/codex.gointernal/handler/websocket.gointernal/repository/sqlite/proxy_upstream_attempt.gotests/e2e/playwright/requests-table-alignment.spec.ts
✅ Files skipped from review due to trivial changes (1)
- tests/e2e/playwright/requests-table-alignment.spec.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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 (7)
internal/handler/codex.go (1)
149-157: 租户参数透传到 OAuth 会话创建链路这部分是正确的。Line 149 与 Line 157 将
tenantID明确传入StartOAuth/CreateSession，与本次多租户隔离修复目标一致。internal/repository/sqlite/proxy_upstream_attempt.go (3)
55-66: 这里的兼容层收敛得很好。
CountAll()统一走CountByTenant(domain.TenantIDAll)后，新旧入口只维护一套计数逻辑，后续再调整租户规则时更不容易漏改旧路径。
70-72: 全量扫描入口的委托也合理。
StreamForCostCalc()现在只是租户版入口的一个特例，避免了两套批处理实现继续分叉。
100-139: 批处理语义保持住了。引入租户过滤后，
id > lastID的游标推进、按批回调以及 callback 出错即短路返回都还在，原有成本重算流程的执行模型没有被顺手改坏。internal/handler/claude.go (1)
105-111: Claude OAuth 的租户上下文已经贯穿到会话创建路径。这里把
tenantID从 HTTP 入口一路传到CreateSession，能和internal/adapter/provider/claude/service.go里的OAuthSession.TenantID/CompleteSession广播逻辑对齐，修复目标是准确的。Also applies to: 161-162
internal/handler/websocket.go (2)
39-44: 把作用域元数据和消息一起入队这个方案是对的。
tenantID/scoped/defaultOnly统一在队列层携带，再由run()集中筛选收件人，能避免某个广播分支漏掉租户过滤，也和这次更新后的 websocket 测试断言模型保持一致。Also applies to: 75-93, 201-252, 254-283
127-153: 先鉴权再 Upgrade 的顺序正确。未授权请求可以直接返回普通 HTTP 错误，不会先建立连接再清理，也避免了失败连接短暂进入
clients映射。Also applies to: 155-170
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/websocket.go`:
- Around line 132-134: The current WebSocket auth branch returns an admin
wsClient when auth == nil which grants admin privileges by default; change the
behavior to deny by returning wsClient{} and false instead of an admin client,
and add a controlled opt-in for "local-only" anonymous admin access (e.g., check
a new localOnly bool or config flag before returning elevated wsClient) so
anonymous admin is only allowed when explicitly enabled; also ensure the
SetAuthMiddleware setter is actually used during server initialization (wire
authMiddleware into the WebSocket connection logic where wsClient is created) so
deployments without an auth middleware fail closed rather than falling back to
admin.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f718b46f-acc1-4b29-a903-5cadc58dded0
📒 Files selected for processing (3)
internal/handler/codex.gointernal/handler/websocket.gointernal/repository/sqlite/proxy_upstream_attempt.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/handler/codex.go
- internal/repository/sqlite/proxy_upstream_attempt.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). (2)
- GitHub Check: playwright
- 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-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 (2)
internal/handler/websocket.go (2)
75-113: 广播发送循环的并发处理实现是稳妥的这里先在读锁下复制目标连接，再在锁外写 socket，最后集中清理失效连接，避免了“持锁网络 I/O”和 map 并发删除问题，方向正确。
286-289:BroadcastLog当前实现保持全局广播语义，兼容性正确
BroadcastLog走broadcastJSONMessage(0, false, false, ...)，没有把日志流静默收窄到默认租户，行为与既有接口语义一致。
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/maxx/main.go`:
- Line 378: The call to handler.NewProjectProxyHandler passes four arguments but
the function signature for NewProjectProxyHandler (returning
*ProjectProxyHandler) accepts only three: proxyHandler *ProxyHandler,
modelsHandler *ModelsHandler, projectRepo repository.ProjectRepository; remove
the extra tokenAuthMiddleware argument from the call site (the variable named
tokenAuthMiddleware) or alternatively update the NewProjectProxyHandler function
signature to accept and wire that middleware if intended — adjust either the
call or the function definition (NewProjectProxyHandler / ProjectProxyHandler)
so the parameter lists match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cd06a775-d27d-4b33-a46c-b1686b488c5e
📒 Files selected for processing (3)
cmd/maxx/main.gointernal/handler/websocket.gotests/e2e/playwright/requests-table-alignment.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/playwright/requests-table-alignment.spec.ts
- internal/handler/websocket.go
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🪛 GitHub Actions: E2E Playwright Tests
cmd/maxx/main.go
[error] 378-378: go build failed: too many arguments in call to handler.NewProjectProxyHandler
🪛 GitHub Actions: PR Checks
cmd/maxx/main.go
[error] 378-378: go vet failed: too many arguments in call to handler.NewProjectProxyHandler
🪛 GitHub Check: playwright
cmd/maxx/main.go
[failure] 378-378:
too many arguments in call to handler.NewProjectProxyHandler
🔇 Additional comments (1)
cmd/maxx/main.go (1)
420-422: LGTM!WebSocket 认证中间件的条件注入逻辑正确：仅当
authMiddleware != nil（即启用了管理员认证）时才调用wsHub.SetAuthMiddleware，且在注册/ws端点（第 425 行）之前完成设置，确保后续连接能正确应用认证逻辑。
| // Use already-created cached project repository for project proxy handler | ||
| modelsHandler := handler.NewModelsHandler(responseModelRepo, cachedProviderRepo, cachedModelMappingRepo) | ||
| projectProxyHandler := handler.NewProjectProxyHandler(proxyHandler, modelsHandler, cachedProjectRepo) | ||
| projectProxyHandler := handler.NewProjectProxyHandler(proxyHandler, modelsHandler, cachedProjectRepo, tokenAuthMiddleware) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 查找 NewProjectProxyHandler 的函数定义
ast-grep --pattern 'func NewProjectProxyHandler($$$) $_ {
$$$
}'Repository: awsl-project/maxx
Length of output: 752
修复构建失败：handler.NewProjectProxyHandler 参数过多
第 378 行调用 handler.NewProjectProxyHandler 时传递了 4 个参数，但该函数只接受 3 个参数。函数签名位于 internal/handler/project_proxy.go 第 21-25 行，定义为：
func NewProjectProxyHandler(
proxyHandler *ProxyHandler,
modelsHandler *ModelsHandler,
projectRepo repository.ProjectRepository,
) *ProjectProxyHandler移除第四个参数 tokenAuthMiddleware，或根据需要更新函数签名以接受该参数。
🧰 Tools
🪛 GitHub Actions: E2E Playwright Tests
[error] 378-378: go build failed: too many arguments in call to handler.NewProjectProxyHandler
🪛 GitHub Actions: PR Checks
[error] 378-378: go vet failed: too many arguments in call to handler.NewProjectProxyHandler
🪛 GitHub Check: playwright
[failure] 378-378:
too many arguments in call to handler.NewProjectProxyHandler
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/maxx/main.go` at line 378, The call to handler.NewProjectProxyHandler
passes four arguments but the function signature for NewProjectProxyHandler
(returning *ProjectProxyHandler) accepts only three: proxyHandler *ProxyHandler,
modelsHandler *ModelsHandler, projectRepo repository.ProjectRepository; remove
the extra tokenAuthMiddleware argument from the call site (the variable named
tokenAuthMiddleware) or alternatively update the NewProjectProxyHandler function
signature to accept and wire that middleware if intended — adjust either the
call or the function definition (NewProjectProxyHandler / ProjectProxyHandler)
so the parameter lists match.
2 participants
Summary
CountByTenanton the upstream-attempt repository interface for the admin cost recalculation pathWhy
Recent multi-tenant changes left the OAuth session structs and repository interface out of sync with their callers. That broke builds in
internal/serviceand caused OAuth result broadcasts to lose tenant scoping. The websocket tests were also still targeting the old broadcast channel shape.Testing
go test ./...Summary by CodeRabbit
发行说明
新功能
测试