feat: API improvements - config reload, rule providers, log stability#1343
Merged
Conversation
…bility - Add config_path to GlobalState for config reload support - Add rule provider routes (/providers/rules/*) with list, reload, match endpoints - Wire rule_routes in ApiRunner - Handle lagged broadcast errors in log websocket handler - Add start_idle_listeners/restart_idle to InboundManager to avoid EADDRINUSE on patch - Add get_rule_providers() to Router - Add list_rules()/ruleCount to RuleProvider Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughThis PR enhances configuration management, error handling, and introduces a rule provider API. Changes include adding config path tracking to global state, improving config reload path resolution with better error handling, optimizing restart behavior with a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Comment |
ibigbug
added a commit
that referenced
this pull request
Apr 27, 2026
… API changes Move all non-embed Rust API improvements (config reload, rule provider routes, log lag handling, inbound restart_idle) to feat/api-improvements PR #1343. Dashboard branch now contains only: - Embedded dashboard serving (embedded_dashboard.rs, /ui/* routing) - build.rs npm ci steps - Cargo.toml dashboard feature flag - All clash-dashboard frontend files Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clash-lib/src/app/api/handlers/config.rs (1)
322-357:⚠️ Potential issue | 🔴 CriticalBind-address changes still need a full restart.
If
bind_addresschanged earlier in this handler and any port field is also present,port_changedmakes this branch callrestart_idle(). That only restarts handles cleared bychange_ports(), so listeners whose port did not change keep serving on the old bind address.🛠️ Proposed fix
let inbound_manager = state.inbound_manager.clone(); let mut need_restart = false; + let mut full_restart = false; if let Some(bind_address) = payload.bind_address.clone() { match bind_address.parse::<BindAddress>() { Ok(bind_address) => { inbound_manager.set_bind_address(bind_address).await; need_restart = true; + full_restart = true; } Err(_) => { return ( @@ if let Some(allow_lan) = payload.allow_lan && allow_lan != inbound_manager.get_allow_lan().await { inbound_manager.set_allow_lan(allow_lan).await; // TODO: can be done with AtomicBool in each inbound manager, but requires // more changes need_restart = true; - port_changed = false; // force full restart + full_restart = true; } @@ - if need_restart { - if port_changed { - // Port-only change: restart only the affected listener(s). - // Unchanged listeners keep running — no EADDRINUSE. - let _ = inbound_manager.restart_idle().await; - } else { - let _ = inbound_manager.restart().await; - } + if full_restart { + let _ = inbound_manager.restart().await; + } else if port_changed { + // Port-only change: restart only the affected listener(s). + // Unchanged listeners keep running — no EADDRINUSE. + let _ = inbound_manager.restart_idle().await; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clash-lib/src/app/api/handlers/config.rs` around lines 322 - 357, If the bind_address was changed earlier, a port-only restart is unsafe; detect when payload.bind_address is Some and differs from inbound_manager.get_bind_address().await and treat it like a full restart by setting need_restart = true and port_changed = false (similar to the allow_lan branch). Update the handler to compare payload.bind_address against inbound_manager.get_bind_address().await after applying any bind_address change (or when deciding restarts), and if different set inbound_manager.set_bind_address(...) as needed, then force a full restart by clearing port_changed so code uses inbound_manager.restart() instead of restart_idle().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clash-lib/src/app/inbound/manager.rs`:
- Around line 384-425: stop_all_listeners currently aborts and clears
provider-owned listener tasks (entries in provider_handles) but restart() only
re-binds static inbound_handlers via start_all_listeners, so provider listeners
stay down after a full restart; update restart() (and the code path referenced
around the other occurrence at the 470-484 area) to also restart provider-owned
listeners by invoking the same startup logic used for provider_handles (e.g.,
call or extract a function like start_provider_listeners or extend
start_all_listeners to iterate provider_handles and recreate/respawn their tasks
from the provider_handles entries), ensuring you reference provider_handles,
entry.handle, and the existing start_all_listeners/stop_all_listeners functions
so provider inbounds are re-bound after a restart.
- Around line 614-621: The closure passed to extract_if currently treats "option
present" as a change and returns true whenever a port option exists; update it
to compare the new optional port value against the current ports stored in
ports.port / ports.socks_port / ports.mixed_port / ports.tproxy_port /
ports.redir_port and only return true if the option is Some(value) and the value
differs from the existing port. Modify the match arms for InboundOpts::Http,
::Socks, ::Mixed, ::TProxy, and ::Redir in extract_if (and the similar blocks
around lines 626–674) to inspect the inner port (e.g. port.map(|p| Some(p) !=
ports.port).unwrap_or(false) or equivalent) rather than just checking
ports.*_port.is_some().
In `@clash-lib/src/lib.rs`:
- Line 126: GlobalState.config_path is not updated when a file-based reload
occurs; locate where configs are loaded from a path (e.g., the PUT /configs
handler and the reload loop functions that call the file loader) and set
GlobalState.config_path = Some(new_path.clone()) whenever a successful
load-from-file happens. Update the code paths that perform
reload_from_file/load_config_from_path (and any functions around lines 138-142
and 211-240 handling reloads) to mutate the GlobalState instance (the struct
with config_path) after the load succeeds so subsequent empty-path reloads use
the new path.
---
Outside diff comments:
In `@clash-lib/src/app/api/handlers/config.rs`:
- Around line 322-357: If the bind_address was changed earlier, a port-only
restart is unsafe; detect when payload.bind_address is Some and differs from
inbound_manager.get_bind_address().await and treat it like a full restart by
setting need_restart = true and port_changed = false (similar to the allow_lan
branch). Update the handler to compare payload.bind_address against
inbound_manager.get_bind_address().await after applying any bind_address change
(or when deciding restarts), and if different set
inbound_manager.set_bind_address(...) as needed, then force a full restart by
clearing port_changed so code uses inbound_manager.restart() instead of
restart_idle().
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f4a973e1-5ac7-4091-9b6f-28b8528a000a
📒 Files selected for processing (9)
clash-lib/src/app/api/handlers/config.rsclash-lib/src/app/api/handlers/log.rsclash-lib/src/app/api/handlers/provider.rsclash-lib/src/app/api/runner.rsclash-lib/src/app/inbound/manager.rsclash-lib/src/app/remote_content_manager/providers/rule_provider/mrs.rsclash-lib/src/app/remote_content_manager/providers/rule_provider/provider.rsclash-lib/src/app/router/mod.rsclash-lib/src/lib.rs
Comment on lines
+384
to
425
| async fn stop_all_listeners(&self) { | ||
| let mut handles_to_await: Vec<JoinHandle<()>> = Vec::new(); | ||
|
|
||
| // Abort static inbound handlers, collect handles for awaiting. | ||
| { | ||
| let mut guard = self.inbound_handlers.write().await; | ||
| for (opt, l) in guard.iter_mut() { | ||
| if let Some(handler) = l.take() { | ||
| warn!( | ||
| "Shutting down provider inbound handler: {}", | ||
| "Shutting down inbound handler: {}", | ||
| opt.common_opts().name | ||
| ); | ||
| h.abort(); | ||
| handler.abort(); | ||
| handles_to_await.push(handler); | ||
| } | ||
| *l = None; | ||
| } | ||
| } | ||
|
|
||
| // Abort provider inbound handlers. | ||
| { | ||
| let mut provider_guard = self.provider_handles.write().await; | ||
| for handles in provider_guard.values_mut() { | ||
| for (opt, entry) in handles.iter_mut() { | ||
| if let Some(h) = entry.handle.take() { | ||
| warn!( | ||
| "Shutting down provider inbound handler: {}", | ||
| opt.common_opts().name | ||
| ); | ||
| h.abort(); | ||
| handles_to_await.push(h); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Wait for all aborted tasks to finish so the OS ports are released | ||
| // before new listeners bind to the same addresses. | ||
| for h in handles_to_await { | ||
| let _ = h.await; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Full restart currently drops provider-owned listeners.
stop_all_listeners() now aborts entries from provider_handles, but restart() only calls start_all_listeners() for inbound_handlers. After any full restart, provider inbounds stay down until their provider happens to emit another update.
Also applies to: 470-484
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clash-lib/src/app/inbound/manager.rs` around lines 384 - 425,
stop_all_listeners currently aborts and clears provider-owned listener tasks
(entries in provider_handles) but restart() only re-binds static
inbound_handlers via start_all_listeners, so provider listeners stay down after
a full restart; update restart() (and the code path referenced around the other
occurrence at the 470-484 area) to also restart provider-owned listeners by
invoking the same startup logic used for provider_handles (e.g., call or extract
a function like start_provider_listeners or extend start_all_listeners to
iterate provider_handles and recreate/respawn their tasks from the
provider_handles entries), ensuring you reference provider_handles,
entry.handle, and the existing start_all_listeners/stop_all_listeners functions
so provider inbounds are re-bound after a restart.
Comment on lines
614
to
+621
| .extract_if(|opts, _| match &opts { | ||
| InboundOpts::Http { common_opts } => { | ||
| ports.port.is_some() && Some(common_opts.port) == ports.port | ||
| } | ||
| InboundOpts::Socks { common_opts, .. } => { | ||
| ports.socks_port.is_some() | ||
| && Some(common_opts.port) == ports.socks_port | ||
| } | ||
| InboundOpts::Mixed { common_opts, .. } => { | ||
| ports.mixed_port.is_some() | ||
| && Some(common_opts.port) == ports.mixed_port | ||
| } | ||
| InboundOpts::Http { .. } => ports.port.is_some(), | ||
| InboundOpts::Socks { .. } => ports.socks_port.is_some(), | ||
| InboundOpts::Mixed { .. } => ports.mixed_port.is_some(), | ||
| #[cfg(feature = "tproxy")] | ||
| InboundOpts::TProxy { common_opts, .. } => { | ||
| ports.tproxy_port.is_some() | ||
| && Some(common_opts.port) == ports.tproxy_port | ||
| } | ||
| InboundOpts::TProxy { .. } => ports.tproxy_port.is_some(), | ||
| #[cfg(feature = "redir")] | ||
| InboundOpts::Redir { common_opts } => { | ||
| ports.redir_port.is_some() | ||
| && Some(common_opts.port) == ports.redir_port | ||
| } | ||
| InboundOpts::Redir { .. } => ports.redir_port.is_some(), |
Contributor
There was a problem hiding this comment.
Ignore no-op port patches before aborting listeners.
This now treats “field present” as “port changed”. Sending the same port value still aborts the task, waits for shutdown, and makes the caller believe a restart was required, which causes avoidable listener churn.
♻️ Proposed fix
let listeners: HashMap<InboundOpts, Option<_>> = guard
.extract_if(|opts, _| match &opts {
- InboundOpts::Http { .. } => ports.port.is_some(),
- InboundOpts::Socks { .. } => ports.socks_port.is_some(),
- InboundOpts::Mixed { .. } => ports.mixed_port.is_some(),
+ InboundOpts::Http { common_opts } => {
+ ports.port.is_some_and(|p| p != common_opts.port)
+ }
+ InboundOpts::Socks { common_opts, .. } => {
+ ports.socks_port.is_some_and(|p| p != common_opts.port)
+ }
+ InboundOpts::Mixed { common_opts, .. } => {
+ ports.mixed_port.is_some_and(|p| p != common_opts.port)
+ }
#[cfg(feature = "tproxy")]
- InboundOpts::TProxy { .. } => ports.tproxy_port.is_some(),
+ InboundOpts::TProxy { common_opts, .. } => {
+ ports.tproxy_port.is_some_and(|p| p != common_opts.port)
+ }
#[cfg(feature = "redir")]
- InboundOpts::Redir { .. } => ports.redir_port.is_some(),
+ InboundOpts::Redir { common_opts } => {
+ ports.redir_port.is_some_and(|p| p != common_opts.port)
+ }
_ => false,
})
.collect();Also applies to: 626-674
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clash-lib/src/app/inbound/manager.rs` around lines 614 - 621, The closure
passed to extract_if currently treats "option present" as a change and returns
true whenever a port option exists; update it to compare the new optional port
value against the current ports stored in ports.port / ports.socks_port /
ports.mixed_port / ports.tproxy_port / ports.redir_port and only return true if
the option is Some(value) and the value differs from the existing port. Modify
the match arms for InboundOpts::Http, ::Socks, ::Mixed, ::TProxy, and ::Redir in
extract_if (and the similar blocks around lines 626–674) to inspect the inner
port (e.g. port.map(|p| Some(p) != ports.port).unwrap_or(false) or equivalent)
rather than just checking ports.*_port.is_some().
| dns_listener: ArcRunner, | ||
| reload_tx: mpsc::Sender<(Config, oneshot::Sender<()>)>, | ||
| cwd: String, | ||
| config_path: Option<String>, |
Contributor
There was a problem hiding this comment.
Keep GlobalState.config_path in sync after file-based reloads.
This is initialized from the startup input, and GET /configs plus empty-path reloads now rely on it, but the reload loop never updates it when PUT /configs loads a different file. After one successful reload-from-file, the next empty-path reload still points at the old config.
Also applies to: 138-142, 211-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clash-lib/src/lib.rs` at line 126, GlobalState.config_path is not updated
when a file-based reload occurs; locate where configs are loaded from a path
(e.g., the PUT /configs handler and the reload loop functions that call the file
loader) and set GlobalState.config_path = Some(new_path.clone()) whenever a
successful load-from-file happens. Update the code paths that perform
reload_from_file/load_config_from_path (and any functions around lines 138-142
and 211-240 handling reloads) to mutate the GlobalState instance (the struct
with config_path) after the load succeeds so subsequent empty-path reloads use
the new path.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
📊 Proxy Throughput Results
Tests ran 5 variant(s) in parallel; each direction transfers the full payload. Full test logDownload the |
ibigbug
added a commit
that referenced
this pull request
Apr 27, 2026
… API changes Move all non-embed Rust API improvements (config reload, rule provider routes, log lag handling, inbound restart_idle) to feat/api-improvements PR #1343. Dashboard branch now contains only: - Embedded dashboard serving (embedded_dashboard.rs, /ui/* routing) - build.rs npm ci steps - Cargo.toml dashboard feature flag - All clash-dashboard frontend files Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Apr 28, 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
1 participant
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.
Overview
API improvements for the clash-rs backend, extracted from the dashboard branch.
Changes
Config reload
config_pathtoGlobalStateso the API can track which config file was loadedPUT /configs(reload) now correctly finds the original config file pathRule provider endpoints
GET /providers/rules– list all rule providersPUT /providers/rules/:name– reload a rule providerGET /providers/rules/:name/match?target=<domain|ip>– test if a target matches a rule providerLog stability
Laggedbroadcast errors in log WS handler instead of silently dropping the connectionInboundManager improvements
start_idle_listeners/restart_idleto only restart listeners whose port/address changed, avoidingEADDRINUSEwhen patching config for unchanged portsRouter
get_rule_providers()to expose the rule provider map to API handlersSummary by CodeRabbit
Release Notes
New Features
Bug Fixes