refactor(server): Handler methods take &mut self; Recorder takes env map

Handler trait methods now take `&mut self`. The server wraps `H` in
`RefCell` internally so the N per-client futures that coexist in
`FuturesUnordered` can each call &mut-self methods; on the single-threaded
runtime the borrow check can't fail because handler methods are sync
(no borrow spans an await).

Recorder drops the generic env-lookup closure in favor of a concrete
`FxHashMap<Str, Arc<OsStr>>` taken at construction. Handler impl uses
plain owned fields (no interior mutability inside the handler itself).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: wan9chi

crates/vite_task_server/src/lib.rs

@@ -1,5 +1,5 @@
 use std::{
-    cell::{Cell, RefCell},
+    cell::RefCell,
     ffi::{OsStr, OsString},
     io,
     sync::Arc,
@@ -19,23 +19,23 @@ use vite_str::Str;
 use vite_task_ipc_shared::{IPC_ENV_NAME, Request, Response, ResponseBody};
 
 pub trait Handler {
-    fn ignore_input(&self, path: &Arc<AbsolutePath>);
-    fn ignore_output(&self, path: &Arc<AbsolutePath>);
-    fn disable_cache(&self);
-    fn get_env(&self, name: &str, tracked: bool) -> Option<Arc<OsStr>>;
+    fn ignore_input(&mut self, path: &Arc<AbsolutePath>);
+    fn ignore_output(&mut self, path: &Arc<AbsolutePath>);
+    fn disable_cache(&mut self);
+    fn get_env(&mut self, name: &str, tracked: bool) -> Option<Arc<OsStr>>;
 }
 
-/// A [`Handler`] that records every report and resolves `get_env` via a
-/// user-provided lookup closure.
+/// A [`Handler`] that records every report and resolves `get_env` against
+/// a provided env map.
 ///
 /// Call [`Recorder::into_reports`] after the driver future completes to
 /// recover the collected [`Reports`].
-pub struct Recorder<F> {
-    ignored_inputs: RefCell<FxHashSet<Arc<AbsolutePath>>>,
-    ignored_outputs: RefCell<FxHashSet<Arc<AbsolutePath>>>,
-    cache_disabled: Cell<bool>,
-    env_records: RefCell<FxHashMap<Str, EnvRecord>>,
-    env_lookup: F,
+pub struct Recorder {
+    ignored_inputs: FxHashSet<Arc<AbsolutePath>>,
+    ignored_outputs: FxHashSet<Arc<AbsolutePath>>,
+    cache_disabled: bool,
+    env_records: FxHashMap<Str, EnvRecord>,
+    env_map: FxHashMap<Str, Arc<OsStr>>,
 }
 
 /// A record of an env value requested via `get_env`.
@@ -57,56 +57,49 @@ pub struct Reports {
     pub env_records: FxHashMap<Str, EnvRecord>,
 }
 
-impl<F> Recorder<F>
-where
-    F: Fn(&str) -> Option<Arc<OsStr>>,
-{
-    pub fn new(env_lookup: F) -> Self {
+impl Recorder {
+    #[must_use]
+    pub fn new(env_map: FxHashMap<Str, Arc<OsStr>>) -> Self {
         Self {
-            ignored_inputs: RefCell::default(),
-            ignored_outputs: RefCell::default(),
-            cache_disabled: Cell::new(false),
-            env_records: RefCell::default(),
-            env_lookup,
+            ignored_inputs: FxHashSet::default(),
+            ignored_outputs: FxHashSet::default(),
+            cache_disabled: false,
+            env_records: FxHashMap::default(),
+            env_map,
         }
     }
 
     #[must_use]
     pub fn into_reports(self) -> Reports {
         Reports {
-            ignored_inputs: self.ignored_inputs.into_inner(),
-            ignored_outputs: self.ignored_outputs.into_inner(),
-            cache_disabled: self.cache_disabled.get(),
-            env_records: self.env_records.into_inner(),
+            ignored_inputs: self.ignored_inputs,
+            ignored_outputs: self.ignored_outputs,
+            cache_disabled: self.cache_disabled,
+            env_records: self.env_records,
         }
     }
 }
 
-impl<F> Handler for Recorder<F>
-where
-    F: Fn(&str) -> Option<Arc<OsStr>>,
-{
-    fn ignore_input(&self, path: &Arc<AbsolutePath>) {
-        self.ignored_inputs.borrow_mut().insert(Arc::clone(path));
+impl Handler for Recorder {
+    fn ignore_input(&mut self, path: &Arc<AbsolutePath>) {
+        self.ignored_inputs.insert(Arc::clone(path));
     }
 
-    fn ignore_output(&self, path: &Arc<AbsolutePath>) {
-        self.ignored_outputs.borrow_mut().insert(Arc::clone(path));
+    fn ignore_output(&mut self, path: &Arc<AbsolutePath>) {
+        self.ignored_outputs.insert(Arc::clone(path));
     }
 
-    fn disable_cache(&self) {
-        self.cache_disabled.set(true);
+    fn disable_cache(&mut self) {
+        self.cache_disabled = true;
     }
 
-    fn get_env(&self, name: &str, tracked: bool) -> Option<Arc<OsStr>> {
-        if let Some(existing) = self.env_records.borrow_mut().get_mut(name) {
+    fn get_env(&mut self, name: &str, tracked: bool) -> Option<Arc<OsStr>> {
+        if let Some(existing) = self.env_records.get_mut(name) {
             existing.tracked |= tracked;
             return existing.value.clone();
         }
-        let value = (self.env_lookup)(name);
-        self.env_records
-            .borrow_mut()
-            .insert(name.into(), EnvRecord { tracked, value: value.clone() });
+        let value = self.env_map.get(name).cloned();
+        self.env_records.insert(name.into(), EnvRecord { tracked, value: value.clone() });
         value
     }
 }
@@ -155,8 +148,15 @@ pub fn serve<'h, H: Handler + 'h>(
 
     let run_stop = stop_token.clone();
     let driver = async move {
+        // Multiple per-client futures coexist inside `FuturesUnordered` and each
+        // calls `&mut self` handler methods. `RefCell` provides the interior
+        // mutability that makes these shared-access method calls compile; at
+        // runtime the `borrow_mut()` never conflicts because we're on a
+        // single-threaded runtime and handler methods are synchronous (no
+        // awaits, so no borrow spans a yield point).
+        let handler = RefCell::new(handler);
         run(bound, &handler, run_stop).await;
-        handler
+        handler.into_inner()
     }
     .boxed_local();
 
@@ -203,7 +203,7 @@ const fn listener_of(bound: &Bound) -> &Listener {
     bound
 }
 
-async fn run<H: Handler>(bound: Bound, handler: &H, shutdown: CancellationToken) {
+async fn run<H: Handler>(bound: Bound, handler: &RefCell<H>, shutdown: CancellationToken) {
     let mut clients = FuturesUnordered::new();
 
     // Accept phase: accept new clients until shutdown fires.
@@ -233,7 +233,7 @@ async fn run<H: Handler>(bound: Bound, handler: &H, shutdown: CancellationToken)
     while clients.next().await.is_some() {}
 }
 
-async fn handle_client<H: Handler>(mut stream: Stream, handler: &H) {
+async fn handle_client<H: Handler>(mut stream: Stream, handler: &RefCell<H>) {
     let mut buf = Vec::new();
     loop {
         match read_frame(&mut stream, &mut buf).await {
@@ -256,17 +256,17 @@ async fn handle_client<H: Handler>(mut stream: Stream, handler: &H) {
         match request {
             Request::IgnoreInput(ns) => {
                 if let Some(path) = native_str_to_abs_path(ns) {
-                    handler.ignore_input(&path);
+                    handler.borrow_mut().ignore_input(&path);
                 }
             }
             Request::IgnoreOutput(ns) => {
                 if let Some(path) = native_str_to_abs_path(ns) {
-                    handler.ignore_output(&path);
+                    handler.borrow_mut().ignore_output(&path);
                 }
             }
-            Request::DisableCache => handler.disable_cache(),
+            Request::DisableCache => handler.borrow_mut().disable_cache(),
             Request::GetEnv { id, name, tracked } => {
-                let value = handler.get_env(name, tracked);
+                let value = handler.borrow_mut().get_env(name, tracked);
                 let boxed: Option<Box<NativeStr>> = value.as_deref().map(Into::into);
                 let body = ResponseBody::Env(boxed.as_deref());
                 let response = Response { id, body };

crates/vite_task_server/tests/integration.rs

@@ -7,20 +7,23 @@ use std::{
 use rustc_hash::FxHashMap;
 use tokio::runtime::Builder;
 use vite_path::AbsolutePathBuf;
+use vite_str::Str;
 use vite_task_client::Client;
 use vite_task_server::{Recorder, Reports, ServerHandle, serve};
 
 fn abs(path: &str) -> AbsolutePathBuf {
     AbsolutePathBuf::new(path.into()).expect("absolute path literal")
 }
 
-fn run_with_server<F>(env_map: FxHashMap<&'static str, &'static str>, client_work: F) -> Reports
+fn env_map(pairs: &[(&str, &str)]) -> FxHashMap<Str, Arc<OsStr>> {
+    pairs.iter().map(|(k, v)| (Str::from(*k), Arc::<OsStr>::from(OsStr::new(v)))).collect()
+}
+
+fn run_with_server<F>(envs: FxHashMap<Str, Arc<OsStr>>, client_work: F) -> Reports
 where
     F: FnOnce(OsString) + Send + 'static,
 {
-    let recorder = Recorder::new(move |name: &str| {
-        env_map.get(name).map(|value| Arc::<OsStr>::from(OsStr::new(value)))
-    });
+    let recorder = Recorder::new(envs);
 
     let rt = Builder::new_current_thread().enable_all().build().unwrap();
     rt.block_on(async move {
@@ -41,7 +44,7 @@ where
 
 #[test]
 fn single_client_fire_and_forget() {
-    let reports = run_with_server(FxHashMap::default(), |name| {
+    let reports = run_with_server(env_map(&[]), |name| {
         let mut client = Client::from_name(&name).expect("connect");
         client.ignore_input(abs("/tmp/in.txt").as_absolute_path()).unwrap();
         client.ignore_output(abs("/tmp/out.txt").as_absolute_path()).unwrap();
@@ -57,10 +60,7 @@ fn single_client_fire_and_forget() {
 
 #[test]
 fn get_env_found_and_not_found() {
-    let mut env_map = FxHashMap::default();
-    env_map.insert("NODE_ENV", "production");
-
-    let reports = run_with_server(env_map, |name| {
+    let reports = run_with_server(env_map(&[("NODE_ENV", "production")]), |name| {
         let mut client = Client::from_name(&name).expect("connect");
         let present = client.get_env("NODE_ENV", true).unwrap();
         assert_eq!(present.as_deref(), Some(OsStr::new("production")));
@@ -79,10 +79,7 @@ fn get_env_found_and_not_found() {
 
 #[test]
 fn get_env_tracked_upgrade_is_monotonic() {
-    let mut env_map = FxHashMap::default();
-    env_map.insert("NODE_ENV", "production");
-
-    let reports = run_with_server(env_map, |name| {
+    let reports = run_with_server(env_map(&[("NODE_ENV", "production")]), |name| {
         let mut client = Client::from_name(&name).expect("connect");
         let a = client.get_env("NODE_ENV", false).unwrap();
         let b = client.get_env("NODE_ENV", true).unwrap();
@@ -98,10 +95,7 @@ fn get_env_tracked_upgrade_is_monotonic() {
 
 #[test]
 fn concurrent_clients() {
-    let mut env_map = FxHashMap::default();
-    env_map.insert("SHARED", "value");
-
-    let reports = run_with_server(env_map, |name| {
+    let reports = run_with_server(env_map(&[("SHARED", "value")]), |name| {
         let threads: Vec<_> = (0..4)
             .map(|i| {
                 let name = name.clone();