fix: server propagation, settings atomicity, and config defects#511
Merged
Conversation
- servers: propagate AddServersByJSON/ByURL to the running tunnel — return the added ServerList and call AddOutbounds (ignoring ErrTunnelNotConnected), matching AddServers. - settings: hold the lock across the full mutate+save in Set/Clear/Patch so concurrent writers can't lose updates; ClearUser clears all user keys in one atomic Clear. - vpn: drop the discarded newSplitTunnel construction in NewVPNClient; the rule-set file is ensured where it's consumed. - backend: clear a stale server selection in setServers while disconnected; remove the dead location-picker loop and nil-guard the per-server location lookup.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR delivers several targeted defect fixes across server management, settings persistence, VPN split-tunnel setup, and backend server/location handling—primarily to ensure runtime state stays consistent with persisted configuration and to remove dead/ineffective logic.
Changes:
- Propagate servers added via JSON/URL to the live tunnel by returning the added
ServerListand pushing outbounds (ignoringErrTunnelNotConnected). - Improve settings write atomicity by holding the settings mutex across mutate+save, and make user logout clearing a single persisted operation.
- Remove an unused split-tunnel construction site and ensure the split-tunnel rule file exists where options are built; simplify backend location/server construction and clear stale selection when disconnected.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vpn/vpn.go | Removes unused split-tunnel initialization during VPN client construction. |
| vpn/boxoptions.go | Ensures split-tunnel rule file exists at the point sing-box options are assembled. |
| servers/manager.go | Clones stored servers and changes AddServersByJSON/URL to return a *ServerList for downstream propagation. |
| servers/manager_test.go | Updates tests to use returned ServerList/Tags() from AddServersByJSON/URL. |
| common/settings/settings.go | Holds mutex across Set/Clear/Patch + save; makes Clear variadic and persisted. |
| backend/radiance.go | Refactors server/location assembly, reintroduces AddServers with tunnel propagation, and clears stale selection while disconnected. |
| account/user.go | Switches logout clearing to a single multi-key settings clear operation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WendelHime
approved these changes
Jun 5, 2026
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
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.
Summary
Independent, self-contained defect fixes:
AddServersByJSON/AddServersByURLnow propagate added servers to a running tunnel — they return the added*ServerListand callAddOutbounds(ignoringErrTunnelNotConnected), matchingAddServers. Previously the servers were persisted but never pushed to the live tunnel.Set/Clear/Patchholdk.muacross the full mutate+save, closing a lost-update window between concurrent writers.ClearUserclears all user keys in a single atomicClear, so logout is all-or-nothing rather than seven separate saves.newSplitTunnel(...)construction inNewVPNClient; the rule-set file is ensured where it's consumed.setServersclears a stale server selection while disconnected; removed a dead location-picker loop (its entries were written to a map only ever read by tag) and nil-guarded the per-server location lookup.