Skip to content

Commit d672e25

Browse files
maxholmanclaude
andcommitted
refactor: merge hint and command channels, add From/Display for stats
Replace the separate hint watch channel and command queue with a single mpsc channel carrying NodeCommand (Role, Connect, Listen, Disconnect). Eliminates the Mutex<VecDeque> + Notify on SharedNodeState and the watch<Option<RoleHint>> — one channel handles all Handler→mode-task communication. Add From<Metrics> for proto StatsResponse and Display for StatsResponse so IPC dispatch and MCP output use traits instead of field-by-field mapping. Add From<proto::StatsResponse> for the REST StatsResponse. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 957990f commit d672e25

6 files changed

Lines changed: 204 additions & 205 deletions

File tree

crates/api/src/handlers.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,10 @@ pub async fn stats(State(state): State<ApiState>) -> Result<Json<StatsResponse>,
266266
.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
267267

268268
match resp.response {
269-
Some(management_response::Response::Stats(s)) => Ok(Json(StatsResponse::from(s))),
269+
Some(management_response::Response::Stats(s)) => {
270+
let response: StatsResponse = s.into();
271+
Ok(Json(response))
272+
}
270273
_ => Err(StatusCode::INTERNAL_SERVER_ERROR),
271274
}
272275
}

crates/core/src/control/handler.rs

Lines changed: 76 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@
55
66
use std::{net::SocketAddr, sync::Arc, time::Instant};
77

8-
use std::collections::VecDeque;
9-
108
use arc_swap::ArcSwap;
11-
use tokio::sync::watch;
9+
use tokio::sync::mpsc;
1210
use wallhack_wire::{
1311
control::{
1412
ControlRequest, ControlResponse, ErrorResponse, PeerInfo, PingResponse, RouteInfo,
@@ -38,9 +36,15 @@ fn reply_channel<T>() -> (
3836
std::sync::mpsc::sync_channel(1)
3937
}
4038

41-
/// A command sent from the handler/API layer to the mode task.
39+
/// A command sent from the handler/API layer to the mode task via the
40+
/// control watch channel.
4241
#[derive(Debug)]
4342
pub enum NodeCommand {
43+
/// Set or clear the role hint.
44+
Role {
45+
/// `Some` to set a hint, `None` to clear (auto).
46+
hint: Option<RoleHint>,
47+
},
4448
/// Connect to a remote peer at the given address.
4549
Connect {
4650
/// Target address (host, host:port, etc.).
@@ -73,53 +77,31 @@ struct NodeState {
7377
peer_addr: Option<String>,
7478
}
7579

76-
/// Inner state backing [`SharedNodeState`].
77-
struct SharedNodeStateInner {
78-
state: ArcSwap<NodeState>,
79-
commands: std::sync::Mutex<VecDeque<NodeCommand>>,
80-
command_notify: tokio::sync::Notify,
81-
}
82-
8380
/// Shared node state handle, cloneable and cheaply updatable.
8481
///
8582
/// Consumers call [`SharedNodeState::update_role`], [`SharedNodeState::update_capabilities`],
8683
/// etc. after negotiation or listening starts so that `wallhack info`
8784
/// reflects the real state of the daemon.
88-
///
89-
/// Also hosts the command queue for dynamic connect/listen/disconnect
90-
/// operations, avoiding a dedicated channel between handler and mode task.
91-
#[derive(Clone)]
92-
pub struct SharedNodeState(Arc<SharedNodeStateInner>);
93-
94-
impl std::fmt::Debug for SharedNodeState {
95-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
96-
f.debug_struct("SharedNodeState")
97-
.field("state", &*self.0.state.load())
98-
.finish()
99-
}
100-
}
85+
#[derive(Clone, Debug)]
86+
pub struct SharedNodeState(Arc<ArcSwap<NodeState>>);
10187

10288
impl SharedNodeState {
10389
fn new(role: NodeRole) -> Self {
104-
Self(Arc::new(SharedNodeStateInner {
105-
state: ArcSwap::from_pointee(NodeState {
106-
role,
107-
capabilities: Capabilities::default(),
108-
listen_addr: None,
109-
peer_addr: None,
110-
}),
111-
commands: std::sync::Mutex::new(VecDeque::new()),
112-
command_notify: tokio::sync::Notify::new(),
113-
}))
90+
Self(Arc::new(ArcSwap::from_pointee(NodeState {
91+
role,
92+
capabilities: Capabilities::default(),
93+
listen_addr: None,
94+
peer_addr: None,
95+
})))
11496
}
11597

11698
fn load(&self) -> arc_swap::Guard<Arc<NodeState>> {
117-
self.0.state.load()
99+
self.0.load()
118100
}
119101

120102
/// Update the node role (e.g. after auto-negotiation resolves).
121103
pub fn update_role(&self, role: NodeRole) {
122-
self.0.state.rcu(|old| {
104+
self.0.rcu(|old| {
123105
let mut new = (**old).clone();
124106
new.role = role;
125107
new
@@ -128,7 +110,7 @@ impl SharedNodeState {
128110

129111
/// Update the node's own capabilities.
130112
pub fn update_capabilities(&self, capabilities: Capabilities) {
131-
self.0.state.rcu(|old| {
113+
self.0.rcu(|old| {
132114
let mut new = (**old).clone();
133115
new.capabilities = capabilities;
134116
new
@@ -137,47 +119,13 @@ impl SharedNodeState {
137119

138120
/// Record that the node is now listening on `addr`.
139121
pub fn set_listen_addr(&self, addr: SocketAddr) {
140-
self.0.state.rcu(|old| {
122+
self.0.rcu(|old| {
141123
let mut new = (**old).clone();
142124
new.listen_addr = Some(addr);
143125
new.capabilities.listening = true;
144126
new
145127
});
146128
}
147-
148-
/// Enqueue a command for the mode task and wake it.
149-
///
150-
/// # Panics
151-
///
152-
/// Panics if the command queue mutex is poisoned.
153-
pub fn push_command(&self, cmd: NodeCommand) {
154-
self.0
155-
.commands
156-
.lock()
157-
.expect("command queue mutex poisoned")
158-
.push_back(cmd);
159-
self.0.command_notify.notify_one();
160-
}
161-
162-
/// Drain all pending commands from the queue.
163-
///
164-
/// # Panics
165-
///
166-
/// Panics if the command queue mutex is poisoned.
167-
#[must_use]
168-
pub fn drain_commands(&self) -> Vec<NodeCommand> {
169-
self.0
170-
.commands
171-
.lock()
172-
.expect("command queue mutex poisoned")
173-
.drain(..)
174-
.collect()
175-
}
176-
177-
/// Wait until at least one command is enqueued.
178-
pub async fn notified(&self) {
179-
self.0.command_notify.notified().await;
180-
}
181129
}
182130

183131
/// Configuration for the control handler.
@@ -209,9 +157,12 @@ impl HandlerConfig {
209157
/// on the current state of metrics and configuration.
210158
pub struct Handler {
211159
config: HandlerConfig,
212-
/// Sender for hint changes. The mode task watches the receiver and
213-
/// re-evaluates when a new hint arrives. `None` means no hint is active.
214-
hint_tx: watch::Sender<Option<RoleHint>>,
160+
/// Command channel to the mode task. Carries role changes, connect,
161+
/// listen, and disconnect commands.
162+
directive_source: mpsc::Sender<NodeCommand>,
163+
/// Receiver side of the command channel. Extracted once by the daemon
164+
/// before wrapping Handler in `Arc<dyn NodeApi>`.
165+
directive_sink: std::sync::Mutex<Option<mpsc::Receiver<NodeCommand>>>,
215166
log_buffer: LogBuffer,
216167
metrics: SharedMetrics,
217168
peers: SharedRegistry,
@@ -237,10 +188,11 @@ impl Handler {
237188
log_buffer: Option<LogBuffer>,
238189
) -> Self {
239190
let state = SharedNodeState::new(config.node_role);
240-
let (hint_tx, _) = watch::channel(None);
191+
let (directive_source, directive_sink) = mpsc::channel(8);
241192
Self {
242193
config,
243-
hint_tx,
194+
directive_source,
195+
directive_sink: std::sync::Mutex::new(Some(directive_sink)),
244196
log_buffer: log_buffer.unwrap_or_default(),
245197
metrics,
246198
peers,
@@ -261,10 +213,19 @@ impl Handler {
261213
self.state.clone()
262214
}
263215

264-
/// Returns a receiver that fires when the runtime hint changes.
216+
/// Extracts the command receiver. Called once by the daemon before
217+
/// wrapping Handler in `Arc<dyn NodeApi>`.
218+
///
219+
/// # Panics
220+
///
221+
/// Panics if the mutex is poisoned or if called more than once.
265222
#[must_use]
266-
pub fn hint_rx(&self) -> watch::Receiver<Option<RoleHint>> {
267-
self.hint_tx.subscribe()
223+
pub fn directive_sink(&self) -> mpsc::Receiver<NodeCommand> {
224+
self.directive_sink
225+
.lock()
226+
.expect("directive_sink mutex poisoned")
227+
.take()
228+
.expect("directive_sink already taken")
268229
}
269230

270231
/// Handles a control request and returns a response.
@@ -518,10 +479,16 @@ impl crate::node_api::NodeApi for Handler {
518479

519480
fn connect(&self, addr: &str) -> crate::node_api::Result<crate::node_api::ConnectInfo> {
520481
let (reply_sender, reply_receiver) = reply_channel();
521-
self.state.push_command(NodeCommand::Connect {
522-
addr: addr.to_string(),
523-
reply: reply_sender,
524-
});
482+
self.directive_source
483+
.try_send(NodeCommand::Connect {
484+
addr: addr.to_string(),
485+
reply: reply_sender,
486+
})
487+
.map_err(|_| {
488+
crate::node_api::NodeApiError::NotSupported(
489+
"dynamic connect not supported in this mode".into(),
490+
)
491+
})?;
525492
tokio::task::block_in_place(|| reply_receiver.recv()).map_err(|_| {
526493
crate::node_api::NodeApiError::Internal("mode task dropped reply".into())
527494
})?
@@ -532,20 +499,32 @@ impl crate::node_api::NodeApi for Handler {
532499
addr: std::net::SocketAddr,
533500
) -> crate::node_api::Result<crate::node_api::ListenInfo> {
534501
let (reply_sender, reply_receiver) = reply_channel();
535-
self.state.push_command(NodeCommand::Listen {
536-
addr,
537-
reply: reply_sender,
538-
});
502+
self.directive_source
503+
.try_send(NodeCommand::Listen {
504+
addr,
505+
reply: reply_sender,
506+
})
507+
.map_err(|_| {
508+
crate::node_api::NodeApiError::NotSupported(
509+
"dynamic listen not supported in this mode".into(),
510+
)
511+
})?;
539512
tokio::task::block_in_place(|| reply_receiver.recv()).map_err(|_| {
540513
crate::node_api::NodeApiError::Internal("mode task dropped reply".into())
541514
})?
542515
}
543516

544517
fn disconnect(&self) -> crate::node_api::Result<()> {
545518
let (reply_sender, reply_receiver) = reply_channel();
546-
self.state.push_command(NodeCommand::Disconnect {
547-
reply: reply_sender,
548-
});
519+
self.directive_source
520+
.try_send(NodeCommand::Disconnect {
521+
reply: reply_sender,
522+
})
523+
.map_err(|_| {
524+
crate::node_api::NodeApiError::NotSupported(
525+
"dynamic disconnect not supported in this mode".into(),
526+
)
527+
})?;
549528
tokio::task::block_in_place(|| reply_receiver.recv()).map_err(|_| {
550529
crate::node_api::NodeApiError::Internal("mode task dropped reply".into())
551530
})?
@@ -620,12 +599,16 @@ impl crate::node_api::NodeApi for Handler {
620599
}
621600

622601
fn hint_set(&self, hint: RoleHint) -> crate::node_api::Result<()> {
623-
self.hint_tx.send_replace(Some(hint));
602+
let _ = self
603+
.directive_source
604+
.try_send(NodeCommand::Role { hint: Some(hint) });
624605
Ok(())
625606
}
626607

627608
fn hint_set_auto(&self) -> crate::node_api::Result<()> {
628-
self.hint_tx.send_replace(None);
609+
let _ = self
610+
.directive_source
611+
.try_send(NodeCommand::Role { hint: None });
629612
Ok(())
630613
}
631614

crates/core/src/ipc.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ fn dispatch_request(request: &ManagementRequest, api: &dyn NodeApi) -> Managemen
304304
}
305305

306306
Some(management_request::Request::Stats(_)) => {
307-
management_response::Response::Stats(StatsResponse::from(api.metrics()))
307+
management_response::Response::Stats(api.metrics().into())
308308
}
309309

310310
Some(management_request::Request::Peers(_)) => {

crates/daemon/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ pub fn start_node(
164164
log_buffer,
165165
);
166166
let node_state = handler.node_state();
167+
let directive_sink = handler.directive_sink();
167168
let node_api: Arc<dyn NodeApi> = Arc::new(handler);
168169

169170
let (shutdown_tx, _shutdown_rx) = watch::channel(());
@@ -177,6 +178,7 @@ pub fn start_node(
177178
route_updates: route_update_rx,
178179
route_updates_tx: route_update_tx,
179180
node_state,
181+
directive_sink,
180182
};
181183
let task = tokio::spawn(async move { mode::run(&config, resources).await.map_err(Into::into) });
182184

0 commit comments

Comments
 (0)