Mutable groups

[garmr-ulfr]

This pull request introduces support for mutable group outbounds, specifically "mutable selector" and "mutable URL test" types, allowing dynamic addition and removal of outbounds from groups at runtime.

New Mutable Group Outbound Support:

• Added MutableOutboundGroup interface in adapter/group.go to support dynamic addition/removal of outbounds in a group.
• Implemented MutableGroupManager in adapter/groups/manager.go to manage mutable groups, providing methods to create, remove, and safely handle outbounds/endpoints with delayed removal logic.
• Introduced MutableSelector outbound in protocol/group/mutableselector.go, allowing runtime selection, addition, and removal of outbounds, with persistent selection and tracing support.
• Added new outbound types TypeMutableSelector and TypeMutableURLTest in constant/proxy.go for registration and identification of mutable group types.
• Defined configuration options for MutableSelectorOutboundOptions and MutableURLTestOutboundOptions in option/group.go.

related to getlantern/engineering/issues/2550

[Copilot]

Pull Request Overview

This pull request introduces support for mutable group outbounds, allowing dynamic addition and removal of outbounds from groups at runtime. The implementation includes new "mutable selector" and "mutable URL test" outbound types with corresponding management infrastructure.

• Adds mutable group functionality with dynamic outbound management capabilities
• Implements new MutableSelector and MutableURLTest outbound types
• Introduces a management system for safely handling delayed outbound removal

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
protocol/group/selector.go	Removed existing selector implementation to be replaced with mutable version
protocol/group/urltest.go	Removed existing URL test implementation to be replaced with mutable version
protocol/group/mutableselector.go	Added new mutable selector outbound with runtime modification capabilities
protocol/group/mutableurltest.go	Added new mutable URL test outbound with dynamic outbound management
option/group.go	Added configuration options for mutable group types
constant/proxy.go	Added new outbound type constants for mutable groups
adapter/group.go	Defined interface for mutable outbound groups
adapter/groups/manager.go	Implemented management system for mutable groups with delayed removal
adapter/groups/manager_test.go	Added tests for the mutable group manager functionality
internal/sync/typed_map.go	Added type-safe concurrent map wrapper for thread-safe operations

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[myleshorton]

The only thing I wondering here is if we could get away with something that just wraps the existing selector and urltest, creating new ones each time and closing the old ones at a higher level. I feel like that might still require the group adaptor, but I just keep mulling over if there might be a simpler way to achieve this.

Code looks generally fine to me, but just seems like a lot of it, and I wonder if we could get away with something sneaky like that.

[garmr-ulfr]

The only thing I wondering here is if we could get away with something that just wraps the existing selector and urltest, creating new ones each time and closing the old ones at a higher level. I feel like that might still require the group adaptor, but I just keep mulling over if there might be a simpler way to achieve this.

Yeah, I thought about that too. The issue is the new ones would have to have different tags and we can't update the routing rules without restarting sing-box.

If you really want to do something cheeky, we could use unsafe to access the outbound fields, but that would add it's own problems, and there would be race conditions.

[WendelHime]

we could use a trace here so we can know which outbound type failed to reach the test URL (but we need to create a event or another trace span inside de go routine since that's happening async)

[garmr-ulfr]

Wouldn't that create a lot of duplicate traces if there's an outbound that can't connect anymore but hasn't been removed yet? Especially for user servers if they never remove the config for it.

That being said, it is a good way to track which servers they can no longer connect to.
What about if it fails X times, add a trace once, then don't trace again unless it succeeds and fails again X times?

[WendelHime]

Added small suggestions but LGTM!