feat: group listener ports (binds inbound ports to proxy groups) - #57
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0862e5d5a4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ...(opts.customProxyGroups ?? []) | ||
| .map((group) => (typeof group.name === "string" ? group.name.trim() : "")) | ||
| .filter(Boolean), |
There was a problem hiding this comment.
When a disabled custom proxy group has the same name as a grouped provider's requested group name, this helper still reserves that disabled name, but generateClashConfig builds the reserved custom set from only enabled groups. Panels that use buildProviderGroupInfo can therefore show/select a suffixed provider group such as ✈️ Air 2, while the generated YAML actually creates ✈️ Air; selecting the displayed name for features like group listeners then points at a group that does not exist in the output.
Useful? React with 👍 / 👎.
| if (list.length === 0) return [] as Array<Record<string, unknown>>; | ||
|
|
||
| const validTargets = new Set(availablePolicyTargets); | ||
| const usedPorts = new Set<number>(Array.isArray(listeners) ? listeners.map((l) => l.port) : []); |
There was a problem hiding this comment.
When the base YAML already declares a listener on the same port as a new group listener, usedPorts only contains generated node-listener ports and mixed-port. The base listeners are merged back in later, so the final YAML can contain two listener entries with the same port, causing the client to fail binding that inbound; seed this set from baseTopLevelPatch.listeners as well before accepting group listener ports.
Useful? React with 👍 / 👎.
| const groupedAttachments = groupedProviderKeys | ||
| .map((key) => attachmentByKey.get(key)) | ||
| .filter((attachment): attachment is ProxyProviderAttachment => Boolean(attachment)); |
There was a problem hiding this comment.
Grouped provider names are assigned after groupedProviderKeys has been derived from the alphabetically sorted provider keys, while the UI/provider plan computes those same names in source attachment order. If two grouped providers request the same group name, or collide with a reserved name, the suffixes can be assigned to different provider keys in the UI versus the generated YAML, so advanced member selections or group listeners saved against one provider key end up pointing at a different airport group.
Useful? React with 👍 / 👎.
| if (typeof config.mixedPort === "number" && Number.isInteger(config.mixedPort)) { | ||
| usedPorts.add(config.mixedPort); |
There was a problem hiding this comment.
When mixed-port is set only in explicit base/DNS YAML, config.mixedPort still holds the default/user-config value, so this check reserves the wrong port. A group listener using the YAML-defined mixed-port is accepted and then merged into the final config alongside that same top-level port, which makes the client fail to bind one of the inbounds.
Useful? React with 👍 / 👎.
| const customKey = typeof item.providerKey === "string" ? item.providerKey.trim() : ""; | ||
| const key = customKey || defaultKey; | ||
| // key 重复时跳过后者（与旧行为一致）；UI 层负责重复提示 | ||
| if (Object.prototype.hasOwnProperty.call(providers, key)) continue; |
There was a problem hiding this comment.
When two URL sources are given the same custom providerKey (or one custom key collides with another source's default key), this branch silently skips the later provider. The new UI exposes free-form key editing but does not block or warn about duplicates, so a user can enable proxy-providers for multiple subscriptions and have one omitted from the generated YAML entirely.
Useful? React with 👍 / 👎.
|
感谢你实现策略组监听端口，这个功能方向我们希望保留，也愿意继续 review。 不过当前 #57 stacked on #56，而我们目前的产品决定是不引入 #56 的 proxy-providers 扩展。因此按现状无法直接合并。能否请你把 #57 调整为一个基于最新 功能范围
UI 调整目前单独的“分组监听管理”区域会继续增加页面密度，希望改为：
状态与生成逻辑
测试希望补充以下回归场景：
如果你愿意按这个范围调整，我们会优先继续 review #57。功能方向本身很有价值，感谢你的贡献。 |
0862e5d to
c3bd42b
Compare
|
Reworked as requested: rebased onto latest ryan/dev as a standalone change (no #56 dependency), stable-ID targets, 127.0.0.1 default with explicit allow-LAN opt-in, unified advanced-settings dialog replacing the separate management section, full conflict checking, and the requested regression scenarios. Ready for another look — thanks! |
|
Thanks for reworking this — the new version is much closer to the requested scope. I confirmed that it is now standalone from #56, uses stable group IDs, defaults listeners to There are still a few blockers before we can merge:
The current PR head also has no GitHub check runs yet. After rebasing and addressing the items above, please trigger the checks and ping us for another review. Thanks again — the feature direction itself looks good. |
Bind a local mixed inbound port to any proxy group (builtin, custom, or
dialer), generating mihomo listeners entries that route traffic through
that group directly.
- core: GroupListenerBinding stores a stable target id ({kind, id})
instead of the display name, so renaming a group keeps its listener
working; resolveGroupListenerEntries validates targets and ports
against the effective mixed-port (including base-YAML overrides),
node listener ports, other group listeners, and base-YAML listeners,
failing generation with a clear error instead of silently skipping
- listeners default to 127.0.0.1; 0.0.0.0 only with an explicit
allow-LAN opt-in; disabled groups pause generation but keep the
binding so re-enabling restores it
- UI: the group-type button in each group row becomes an advanced
settings button opening a save/cancel dialog (group type, load-balance
strategy, listener toggle, port, allow-LAN with security hint); a
status dot marks non-default settings; no extra text in the row
- store action + subscription save/load + auth handoff serialization
- regression tests for all three group kinds, rename stability,
disable/re-enable, conflict sources, deleted targets, and persistence
round-trips
c3bd42b to
af6a4e1
Compare
|
Thanks for the detailed review! All three items are addressed in the updated head (af6a4e1):
The upstream CI run on this head is pending workflow approval — could you approve it? Meanwhile the same commit passed the full CI (lint + unit tests + check:local-app) on my fork: https://github.com/sodiseng/subboost/actions/runs/30323275358 Ready for another review, thanks! |
3 participants
Reworked per maintainer feedback: now a standalone change based on latest
ryan/dev, with no dependency on the proxy-providers extension (#56, closed).Scope
Bind one local mixed inbound port to any proxy group — builtin, custom, or dialer — generating mihomo
listenersentries (proxy: <group>), so traffic hitting that port always exits through that group.Behavior
{kind: module|custom|dialer, id}instead of the display name — renaming a group keeps its listener working.listen: 127.0.0.1.0.0.0.0only via an explicit "allow LAN access" toggle with an amber security hint.mixed-port(including base-YAML overrides), node listener ports, other group listeners, and listeners already present in the base YAML. Invalid targets and port conflicts fail generation/saving with a clear message — never silently skipped.UI
Tests
Regression coverage for: listener generation for all three group kinds; rename stability; disable/pause + re-enable/resume; conflicts with base-YAML
mixed-port/listeners, node listeners, and other bindings; clear error on deleted targets; save/re-edit/subscription restore round-trips; dialog width capped atcalc(100vw-2rem)for narrow screens.