fix: resolve model owned_by from active channels - #4416
Merged
seefs001 merged 2 commits intoMay 21, 2026
Conversation
Contributor
WalkthroughRefactors model listing to compute preferred owner channel types per model (DB query), map channel types to owner names, and build response model objects with ownership and supported endpoint types populated from the resolved ownership instead of global static defaults. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Controller as Controller/ListModels
participant ModelMeta as Model/GetPreferred...
participant DB as Database
participant DTOBuilder as buildOpenAIModel
Client->>Controller: GET /v1/models
activate Controller
Controller->>Controller: Collect model names & groups
Controller->>ModelMeta: GetPreferredModelOwnerChannelTypes(names, groups)
activate ModelMeta
ModelMeta->>DB: Query abilities JOIN channels (filter enabled/groups, order priority/weight/id)
activate DB
DB-->>ModelMeta: preferred channel.type per model
deactivate DB
ModelMeta-->>Controller: map[modelName → channelType]
deactivate ModelMeta
Controller->>Controller: Resolve owner names via channelOwnerName
loop build responses
Controller->>DTOBuilder: buildOpenAIModel(modelId, overrides)
activate DTOBuilder
DTOBuilder-->>Controller: dto.OpenAIModels (OwnedBy, SupportedEndpointTypes)
deactivate DTOBuilder
end
Controller-->>Client: Response with accurate owned_by
deactivate Controller
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controller/model.go (1)
184-243:⚠️ Potential issue | 🟡 Minor
ownerGroupsis not populated in themodelLimitEnablebranch — owner resolution falls back to cross-group.When
modelLimitEnableis true (Line 187),ownerGroupsstays as an empty[]string, so the subsequent call togetPreferredModelOwnersat Line 245 passes emptygroupsintomodel.GetPreferredModelOwnerChannelTypes. In that helper an emptygroupsslice skips the group filter entirely, meaning the preferredchannels.typefor a model can be picked from an ability belonging to a group the current token/user cannot actually route through. That partially defeats the PR goal of makingowned_byreflect the caller's actually-available channels.The token still has a group (via
ContextKeyTokenGroup) and the user has a group — they should also constrain ownership resolution here.🔧 Proposed fix: populate
ownerGroupsin the token-limit branch toouserModelNames := make([]string, 0) ownerGroups := make([]string, 0) modelLimitEnable := common.GetContextKeyBool(c, constant.ContextKeyTokenModelLimitEnabled) if modelLimitEnable { s, ok := common.GetContextKey(c, constant.ContextKeyTokenModelLimit) var tokenModelLimit map[string]bool if ok { tokenModelLimit = s.(map[string]bool) } else { tokenModelLimit = map[string]bool{} } for allowModel, _ := range tokenModelLimit { if !acceptUnsetRatioModel { _, _, exist := ratio_setting.GetModelRatioOrPrice(allowModel) if !exist { continue } } userModelNames = append(userModelNames, allowModel) } + userId := c.GetInt("id") + if userGroup, err := model.GetUserGroup(userId, false); err == nil { + group := userGroup + if tokenGroup := common.GetContextKeyString(c, constant.ContextKeyTokenGroup); tokenGroup != "" { + group = tokenGroup + } + if group == "auto" { + ownerGroups = service.GetUserAutoGroup(userGroup) + } else { + ownerGroups = []string{group} + } + } } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/model.go` around lines 184 - 243, When modelLimitEnable is true the ownerGroups slice is left empty causing downstream owner resolution to ignore group restrictions; fix by populating ownerGroups in that branch the same way it's done in the non-limit branch: read userId via c.GetInt("id") and get the user's group with model.GetUserGroup, read tokenGroup via common.GetContextKeyString(c, constant.ContextKeyTokenGroup), set group = tokenGroup if tokenGroup != "" else the userGroup, then set ownerGroups to service.GetUserAutoGroup(userGroup) when tokenGroup == "auto" or to []string{group} otherwise (use the same symbols: modelLimitEnable, tokenModelLimit, ContextKeyTokenGroup, GetUserGroup, service.GetUserAutoGroup, ownerGroups). Ensure any error handling for GetUserGroup matches the existing branch behavior.
🧹 Nitpick comments (1)
model/model_owner_test.go (1)
47-141: LGTM — table-driven coverage hits priority/weight/tie-break/group/disabled cases.One optional addition worth considering: add a case for empty
groups(to lock in the current behavior or a future change) and emptymodelNames(fast-path early return), so the contract is explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model/model_owner_test.go` around lines 47 - 141, Add two tests to TestGetPreferredModelOwnerChannelTypes: one where groups is an empty slice (e.g., groups: []string{}) to assert current behavior for no group filter using insertPreferredOwnerCandidate/clearPreferredOwnerTables and expecting the appropriate channel selection, and one where modelNames is empty (call GetPreferredModelOwnerChannelTypes with an empty []string{}) to assert the fast-path early return (no error and result map empty). Use the same setup pattern and helper functions (insertPreferredOwnerCandidate, clearPreferredOwnerTables) and assert require.NoError and expected map results so the contract for empty groups and empty modelNames is explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controller/model.go`:
- Around line 113-129: The current channelOwnerName function calls adaptor.Init
which can mutate adaptor state; change the flow to avoid initializing adaptors
when possible by first attempting a non-mutating name lookup (call
adaptor.GetChannelName()) and only call
adaptor.Init(&relaycommon.RelayInfo{...}) as a fallback if the name is empty;
alternatively add and call a new non-mutating method on adaptor implementations
(e.g., GetChannelDisplayName or GetNameWithoutInit) and update channelOwnerName
to use that, and update adaptor implementations (openai, tencent, dify, vertex)
to implement the new non-mutating accessor; keep memoization in
getPreferredModelOwners as-is.
In `@model/model_meta.go`:
- Around line 156-193: GetPreferredModelOwnerChannelTypes currently skips the
group filter when the groups slice is empty which can expose channel types from
groups the caller shouldn't route to; change the function to fail-closed by
returning an empty result when groups is empty: inside
GetPreferredModelOwnerChannelTypes, after normalizing groups (the groups
variable), if len(groups) == 0 return result, nil (or an explicit empty map) so
callers (e.g., ListModels / ownerGroups) must pass authorized groups explicitly;
update any callers if they expect the previous behavior.
---
Outside diff comments:
In `@controller/model.go`:
- Around line 184-243: When modelLimitEnable is true the ownerGroups slice is
left empty causing downstream owner resolution to ignore group restrictions; fix
by populating ownerGroups in that branch the same way it's done in the non-limit
branch: read userId via c.GetInt("id") and get the user's group with
model.GetUserGroup, read tokenGroup via common.GetContextKeyString(c,
constant.ContextKeyTokenGroup), set group = tokenGroup if tokenGroup != "" else
the userGroup, then set ownerGroups to service.GetUserAutoGroup(userGroup) when
tokenGroup == "auto" or to []string{group} otherwise (use the same symbols:
modelLimitEnable, tokenModelLimit, ContextKeyTokenGroup, GetUserGroup,
service.GetUserAutoGroup, ownerGroups). Ensure any error handling for
GetUserGroup matches the existing branch behavior.
---
Nitpick comments:
In `@model/model_owner_test.go`:
- Around line 47-141: Add two tests to TestGetPreferredModelOwnerChannelTypes:
one where groups is an empty slice (e.g., groups: []string{}) to assert current
behavior for no group filter using
insertPreferredOwnerCandidate/clearPreferredOwnerTables and expecting the
appropriate channel selection, and one where modelNames is empty (call
GetPreferredModelOwnerChannelTypes with an empty []string{}) to assert the
fast-path early return (no error and result map empty). Use the same setup
pattern and helper functions (insertPreferredOwnerCandidate,
clearPreferredOwnerTables) and assert require.NoError and expected map results
so the contract for empty groups and empty modelNames is explicit.
🪄 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: fcdb0d5a-891a-4942-ad44-a8bd02a36d01
📒 Files selected for processing (5)
controller/model.gocontroller/model_owned_by_test.gomodel/model_meta.gomodel/model_owner_test.gomodel/task_cas_test.go
Comment on lines
+113
to
+129
| func channelOwnerName(channelType int) string { | ||
| apiType, success := common.ChannelType2APIType(channelType) | ||
| if !success { | ||
| return strings.ToLower(constant.GetChannelTypeName(channelType)) | ||
| } | ||
| adaptor := relay.GetAdaptor(apiType) | ||
| if adaptor == nil { | ||
| return strings.ToLower(constant.GetChannelTypeName(channelType)) | ||
| } | ||
| adaptor.Init(&relaycommon.RelayInfo{ChannelMeta: &relaycommon.ChannelMeta{ | ||
| ChannelType: channelType, | ||
| }}) | ||
| if name := strings.TrimSpace(adaptor.GetChannelName()); name != "" { | ||
| return name | ||
| } | ||
| return strings.ToLower(constant.GetChannelTypeName(channelType)) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect Init implementations across adaptors to confirm they are safe to call with a minimal RelayInfo stub purely to read channel name.
fd -e go 'adaptor.go' relay/channel | xargs -I{} sh -c 'echo "=== {} ==="; rg -n "^func \(a \*Adaptor\) Init\(" -A 15 "{}"'Repository: QuantumNous/new-api
Length of output: 22600
Minor: State mutations in adaptor.Init during per-call channel name lookup
Several adaptors mutate internal state during Init (openai sets ChannelType, tencent calls GetTimestamp(), dify/vertex set mode fields). While these don't affect GetChannelName() output, calling Init on every channel in the response—even with a minimal stub—is indirect. The memoization in getPreferredModelOwners bounds this to distinct channel types, which is acceptable, but consider whether a dedicated method to retrieve channel name without full initialization would be cleaner.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/model.go` around lines 113 - 129, The current channelOwnerName
function calls adaptor.Init which can mutate adaptor state; change the flow to
avoid initializing adaptors when possible by first attempting a non-mutating
name lookup (call adaptor.GetChannelName()) and only call
adaptor.Init(&relaycommon.RelayInfo{...}) as a fallback if the name is empty;
alternatively add and call a new non-mutating method on adaptor implementations
(e.g., GetChannelDisplayName or GetNameWithoutInit) and update channelOwnerName
to use that, and update adaptor implementations (openai, tencent, dify, vertex)
to implement the new non-mutating accessor; keep memoization in
getPreferredModelOwners as-is.
Comment on lines
+156
to
+193
| func GetPreferredModelOwnerChannelTypes(modelNames []string, groups []string) (map[string]int, error) { | ||
| result := make(map[string]int) | ||
| modelNames = normalizeLookupValues(modelNames) | ||
| if len(modelNames) == 0 { | ||
| return result, nil | ||
| } | ||
|
|
||
| type row struct { | ||
| Model string | ||
| ChannelType int | ||
| } | ||
| var rows []row | ||
|
|
||
| query := DB.Table("abilities"). | ||
| Select("abilities.model as model, channels.type as channel_type"). | ||
| Joins("JOIN channels ON abilities.channel_id = channels.id"). | ||
| Where("abilities.model IN ? AND abilities.enabled = ? AND channels.status = ?", modelNames, true, common.ChannelStatusEnabled). | ||
| Order("COALESCE(abilities.priority, 0) DESC"). | ||
| Order("abilities.weight DESC"). | ||
| Order("abilities.channel_id ASC") | ||
|
|
||
| groups = normalizeLookupValues(groups) | ||
| if len(groups) > 0 { | ||
| query = query.Where("abilities."+commonGroupCol+" IN ?", groups) | ||
| } | ||
|
|
||
| if err := query.Scan(&rows).Error; err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| for _, r := range rows { | ||
| if _, ok := result[r.Model]; ok { | ||
| continue | ||
| } | ||
| result[r.Model] = r.ChannelType | ||
| } | ||
| return result, nil | ||
| } |
Contributor
There was a problem hiding this comment.
Minor: when groups is empty the group filter is silently skipped.
If callers ever invoke this with modelNames populated but groups empty (see controller ListModels when modelLimitEnable is true — ownerGroups is never populated in that branch), the query runs without any group constraint and may return a preferred channel type from an ability belonging to a group the current user/token cannot route to. That contradicts the PR's stated goal of reflecting the user's actually-routable channels.
Consider either:
- documenting this as "no group filter when
groupsis empty" and ensuring every call site passes the appropriate groups, or - making empty
groupsreturn an empty map (fail-closed) so controllers must pass groups explicitly.
I'll leave the concrete fix at the call site (see comment on controller/model.go).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@model/model_meta.go` around lines 156 - 193,
GetPreferredModelOwnerChannelTypes currently skips the group filter when the
groups slice is empty which can expose channel types from groups the caller
shouldn't route to; change the function to fail-closed by returning an empty
result when groups is empty: inside GetPreferredModelOwnerChannelTypes, after
normalizing groups (the groups variable), if len(groups) == 0 return result, nil
(or an explicit empty map) so callers (e.g., ListModels / ownerGroups) must pass
authorized groups explicitly; update any callers if they expect the previous
behavior.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
controller/model.go (1)
172-206: Optional:modelListGroups.userGroupfield is unused.
getModelListGroupspopulatesuserGroupin the returned struct, but no caller reads it (ListModelsonly usestokenGroupandownerGroups). Consider dropping the field unless it's reserved for upcoming callers.♻️ Proposed simplification
type modelListGroups struct { - userGroup string tokenGroup string ownerGroups []string } @@ if tokenGroup == "auto" { return modelListGroups{ - userGroup: userGroup, tokenGroup: tokenGroup, ownerGroups: service.GetUserAutoGroup(userGroup), }, nil } @@ return modelListGroups{ - userGroup: userGroup, tokenGroup: tokenGroup, ownerGroups: []string{group}, }, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/model.go` around lines 172 - 206, The modelListGroups.userGroup field is unused by callers (ListModels only uses tokenGroup and ownerGroups); remove the unused userGroup field from the modelListGroups struct and all places that set or return it in getModelListGroups, updating the three struct literals in getModelListGroups to only populate tokenGroup and ownerGroups, and update any code that referenced modelListGroups.userGroup to stop doing so (verify callers like ListModels still compile using tokenGroup and ownerGroups).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@controller/model.go`:
- Around line 172-206: The modelListGroups.userGroup field is unused by callers
(ListModels only uses tokenGroup and ownerGroups); remove the unused userGroup
field from the modelListGroups struct and all places that set or return it in
getModelListGroups, updating the three struct literals in getModelListGroups to
only populate tokenGroup and ownerGroups, and update any code that referenced
modelListGroups.userGroup to stop doing so (verify callers like ListModels still
compile using tokenGroup and ownerGroups).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e45dbf0-3881-4cac-89b6-d4a9e5276cc6
📒 Files selected for processing (2)
controller/model.gocontroller/model_owned_by_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- controller/model_owned_by_test.go
seefs001
approved these changes
May 21, 2026
yiranxiaohui
added a commit
to yiranxiaohui/new-api
that referenced
this pull request
May 25, 2026
Range: 18282e6..3b9ed0a8 (upstream/main as of fetch) Highlights: - feat: support request_header key source (QuantumNous#4903) - feat: Waffo Pancake gateway + admin catalog binding (QuantumNous#4935) - perf: optimize request metadata extraction, drop dead batch helpers in relay/channel/openai/helper.go (QuantumNous#5009) - perf: reduce heap residency for large base64 relay requests - fix(channel): evict auto-disabled multi-key channels from cache (QuantumNous#4983) - fix: resolve model owned_by from active channels (QuantumNous#4416) — introduces channelOwnerName/getPreferredModelOwners/buildOpenAIModel + ListModels refactor - fix: GetAllChannels respects group filter (QuantumNous#4847, QuantumNous#4885) - fix(auth): expose register_enabled, aff_code, localize reset (QuantumNous#4871, QuantumNous#4945, QuantumNous#4769) - fix(webhook): processing + Waffo subscription compliance (QuantumNous#5047, QuantumNous#5038) - refactor(ui): system settings drill-in sidebar + log filter responsiveness Conflicts resolved: - controller/model.go: kept local hiddenMappedModels filter (resolveAccessibleModelGroups + getHiddenMappedModelNamesForGroups) on top of upstream's ListModels refactor; adopted upstream channelOwnerName helper. - relay/channel/openai/helper.go: adopted upstream (HEAD's processChatCompletions/processCompletions were dead code after upstream's perf refactor in QuantumNous#5009). Local patches verified intact: Username + fillTopUpUsernames (model/topup.go, locked by topup_username_test.go), HideUpstreamErrors, Claude developer-role normalization, Gemini role fallback, Model Chat header nav entry, channel affinity auto-clear. Note: go build not run (no Go toolchain in this environment); CI to verify. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
xyfacai
pushed a commit
to xyfacai/new-api
that referenced
this pull request
May 30, 2026
* fix: resolve model owned_by from active channels * fix: respect token group when resolving model owners
SamuelSxy
pushed a commit
to SamuelSxy/new-api-rh
that referenced
this pull request
Jun 7, 2026
* fix: resolve model owned_by from active channels * fix: respect token group when resolving model owners
fx247562340
pushed a commit
to fx247562340/vancine-platform
that referenced
this pull request
Jun 11, 2026
* fix: resolve model owned_by from active channels * fix: respect token group when resolving model owners
330079598
pushed a commit
to 330079598/new-api
that referenced
this pull request
Aug 19, 2026
* fix: resolve model owned_by from active channels * fix: respect token group when resolving model owners
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
2 participants
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.
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
修复
/v1/models返回的owned_by可能不准确的问题。之前
owned_by来自全局静态模型表，同名模型会被后写入的 provider 覆盖，导致返回值不一定符合当前用户实际可用渠道。现在改为基于当前请求可见模型对应的 enabled abilities 和 channels 计算 owner，并按priority -> weight -> channel_id选择主渠道；静态模型表仅作为兜底。这样
/v1/models返回的owned_by会更贴近当前 token / 用户实际可路由的渠道配置，同时不改变响应结构。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
/v1/models返回的owned_by不一定准确 #4398✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
未修改前：
修改后：
Summary by CodeRabbit
New Features
Tests