refactor(core): Removed Arc from sigs where not required

[alexsnaps]

Same as #649, but with up to date main 🤦

The deconstructing of ServerState isn't a huge deal, the incs and decs on the Arc were possibly even DCE'ed at compile time. I have not verified, but I don't think that perf gain there would be of much value. But what it does seem to show is that health_registry (currently?) isn't a "real natural fit" for the ServerState struct.

Should we drop it? Or will we need it eventually?

[alexsnaps]

We can get rid of the deconstruct btw:

diff --git a/server/src/server.rs b/server/src/server.rs
index e35e8c0..efad9cf 100644
--- a/server/src/server.rs
+++ b/server/src/server.rs
@@ -180,22 +180,15 @@ fn spawn_watcher(
     state: ServerState,
 ) -> Option<std::thread::JoinHandle<()>> {
     let path = config_path?;
-    let ServerState {
-        pipelines,
-        health_registry: _,
-        kv_stores,
-        health_shutdown,
-        response_stores,
-    } = state;
     let handle = crate::watcher::spawn_config_watcher(crate::watcher::WatcherParams {
         config_path: path,
-        health_shutdown,
+        health_shutdown: state.health_shutdown,
         initial_config: config,
-        kv_stores,
-        pipelines,
+        kv_stores: state.kv_stores,
+        pipelines: state.pipelines,
         registry: Arc::new(registry),
         #[cfg(feature = "ai-inference")]
-        response_stores,
+        response_stores: state.response_stores,
         shutdown: CancellationToken::new(),
     });
     Some(handle)

I put it in to highlight the health_registry: _

[alexsnaps]

So, yeah as per my comment, I'd just inline the moves in the WatcherParams construction below.
But, do we want to keep health_registry as a field here?

[praxis-bot]

PR Review

Summary: Removes unnecessary Arc::clone calls in register_protocols and spawn_watcher by destructuring ServerState and narrowing the pipelines parameter type.

Overall: The intent is sound -- eliminating redundant Arc ref-count bumps. However, the destructuring introduces a conditional-compilation bug and drops the health_registry silently.

Severity	Count
Critical	1
Medium	1

[praxis-bot]

[Critical] response_stores is destructured unconditionally, but the ServerState field is behind #[cfg(feature = "ai-inference")] (line 111-112). This will fail to compile with --no-default-features. The original code avoided this because state.response_stores was only accessed inside the #[cfg]-gated field initializer on line 198.

Add the cfg gate to the destructuring:

let ServerState {
    pipelines,
    health_registry: _,
    kv_stores,
    health_shutdown,
    #[cfg(feature = "ai-inference")]
    response_stores,
} = state;

[alexsnaps]

Yes, I will not keep the explicit destructure... See description

[praxis-bot]

[Medium] health_registry: _ silently drops the registry. The PR description asks whether health_registry should be removed from ServerState -- but since run_server_with_registry still reads state.health_registry on line 87 (for register_admin_endpoints) and line 86 won't compile if the field is removed, it must stay. The silent drop here is fine for now, but consider adding a short // consumed by register_admin_endpoints before this point comment to explain why it's discarded, since the PR description itself flags uncertainty about this field's purpose.