fix: server icons survive a disk cleaner, and an allow list keeps Plug's search tool

CHANGELOG.md

@@ -5,6 +5,27 @@ All notable changes to plug are documented here.
 The format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Fixed
+
+- Server icons no longer turn into a letter in a colored square after a disk
+  cleaner runs. Found icons are kept with Plug's own files, not in the
+  system cache a cleaner empties, and a search cut short while Plug starts
+  is no longer remembered as "this site has no icon" for a day.
+- A client with an allow list keeps Plug's own search tool, so it can still
+  find the tools it is allowed when Plug serves tools by search.
+- Removing a server takes it off every client's allow list, so a server
+  added later under the same name is not handed to those clients unasked.
+- A sign-in ended because its renewal token was used twice stays ended even
+  when Plug could not save the change.
+- An allowed tool Plug no longer lists is shown as `server/tool`, the form
+  `plug clients only --remove --tool` takes.
+- A watch on a tool Plug does not list right now is hidden from a client
+  whose allow list does not name that tool.
+- The large result of a task called by its server's own tool name is no
+  longer kept from the client that ran it.
+
 ## [0.11.0] - 2026-10-07
 
 ### Added

PlugApp/PlugApp/Core/AppIcons.swift

@@ -440,10 +440,9 @@ final class IconStore {
     init(chosenDirectory: URL? = nil, cacheDirectory: URL? = nil) {
         self.chosenDirectory = chosenDirectory
             ?? PlugIPCClient.defaultSocketURL.deletingLastPathComponent().appending(path: "icons")
-        self.cacheDirectory = cacheDirectory
-            ?? FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask)[0]
-            .appending(path: Bundle.main.bundleIdentifier ?? "com.cyberpapiii.plug")
-            .appending(path: "icons")
+        // Beside the chosen icons, not under Caches: a disk cleaner empties
+        // Caches, and every icon was then looked for again the next day.
+        self.cacheDirectory = cacheDirectory ?? self.chosenDirectory.appending(path: "found")
         let files = (try? FileManager.default.contentsOfDirectory(atPath: self.chosenDirectory.path)) ?? []
         for file in files where file.hasSuffix(".png") {
             chosen[String(file.dropLast(4))] = NSImage(contentsOf: self.chosenDirectory.appending(path: file))
@@ -573,7 +572,10 @@ final class IconStore {
                     switch kept(key) {
                     case let .found(image): advertised[source.name] = image
                     case .missing: break
-                    case .unknown: advertised[source.name] = keep(await SiteIcon.image(at: url), as: key)
+                    case .unknown:
+                        let data = await SiteIcon.image(at: url)
+                        guard !stopped(asking: [key]) else { return }
+                        advertised[source.name] = keep(data, as: key)
                     }
                 }
             }
@@ -610,11 +612,21 @@ final class IconStore {
             for await (index, data) in group { answers[index] = data }
             return answers
         }
+        guard !stopped(asking: asking.map(\.key)) else { return }
         for (index, want) in asking.enumerated() {
             if let image = keep(answers[index], as: want.key) { sites[want.name] = image }
         }
     }
 
+    /// Whether this search was cancelled, as it is when the servers change
+    /// while it runs. A cancelled request comes back empty, which is not a
+    /// site without an icon: nothing is remembered and `keys` are asked again.
+    private func stopped(asking keys: [String]) -> Bool {
+        guard Task.isCancelled else { return false }
+        searched.subtract(keys)
+        return true
+    }
+
     private enum Kept {
         case found(NSImage)
         /// Looked for not long ago, and not there.

PlugApp/PlugAppTests/SettingsSurfaceTests.swift

@@ -385,6 +385,23 @@ final class FoundIconTests: XCTestCase {
         XCTAssertNil(IconStore(chosenDirectory: directory, cacheDirectory: directory).image(forServer: "No Such Server"))
     }
 
+    /// The servers change while icons are being looked for at launch, which
+    /// cancels the search. That used to be remembered as "no icon" for a day.
+    @MainActor
+    func testACancelledIconSearchIsNotRememberedAsNoIcon() async throws {
+        let directory = FileManager.default.temporaryDirectory.appending(path: "plug-icons-\(UUID().uuidString)")
+        defer { try? FileManager.default.removeItem(at: directory) }
+        let found = directory.appending(path: "found")
+        let store = IconStore(chosenDirectory: directory, cacheDirectory: found)
+        let search = Task { @MainActor in
+            await store.load([IconSource(name: "nowhere", address: "https://icons.invalid/mcp")])
+        }
+        search.cancel()
+        await search.value
+        let kept = (try? FileManager.default.contentsOfDirectory(atPath: found.path)) ?? []
+        XCTAssertEqual(kept, [], "nothing is remembered about a search that was cut short")
+    }
+
     func testTheStatusAnswerCarriesWhatAServerSaysAboutItself() throws {
         let json = """
         {"type":"Status","servers":[

plug-core/src/downstream_oauth/mod.rs

@@ -1646,7 +1646,9 @@ impl DownstreamOauthManager {
                 }
                 // Before the save, as for a removed client: if the save
                 // fails, what is already open under this sign-in still ends.
-                if let Some((_, sign_in)) = self.family_ended.remove(&consumed.family_id) {
+                // The entry stays, so a token the failed save left behind
+                // opens nothing new either.
+                if let Some(sign_in) = self.family_ended.get(&consumed.family_id) {
                     sign_in.ended.store(true, Ordering::SeqCst);
                 }
                 let mut next = guard.clone();

plug-core/src/events/access.rs

@@ -58,7 +58,7 @@ impl WatchAccess for RuntimeWatchAccess {
     fn may_use(&self, client_id: &str, watch: &WatchConfig) -> bool {
         self.engine.upgrade().is_some_and(|engine| {
             engine.tool_router().client_may_watch(
-                Some(&format!("oauth:{client_id}")),
+                Some(&crate::ipc::grant_client_key(client_id)),
                 &watch.server,
                 &watch.tool,
             )

plug-core/src/operator.rs

@@ -186,6 +186,14 @@ pub fn apply_operator_mutation_knowing(
             if config.servers.remove(&name).is_none() {
                 anyhow::bail!("unknown server `{name}`");
             }
+            // Off every allow list too, so a server added later under the
+            // same name is a new one and no client gets it unasked.
+            for settings in config.clients.values_mut() {
+                // A list this empties still holds: nothing, not everything.
+                settings.only_allowed = settings.has_allow_list();
+                settings.allowed_servers.retain(|server| *server != name);
+                settings.allowed_tools.retain(|tool| tool.server != name);
+            }
             OperatorMutationResult::server(None)
         }
         OperatorMutation::AddAccount { server, account } => {
@@ -1093,6 +1101,28 @@ API_KEY = "sk-live-123"
         assert!(!config.clients.contains_key("radar"));
     }
 
+    #[test]
+    fn removing_a_server_takes_it_off_every_allow_list() {
+        let path = fixture_path();
+        std::fs::write(
+            &path,
+            "[servers.git]\ncommand = \"git-mcp\"\n\n[clients.radar]\nallowed_servers = [\"git\"]\n",
+        )
+        .unwrap();
+        let (config, _) = apply_operator_mutation_knowing(
+            &path,
+            OperatorMutation::RemoveServer {
+                name: "git".to_string(),
+            },
+            &|_| None,
+        )
+        .unwrap();
+        // A server added later under the same name is not one it gets, and
+        // the emptied list still holds.
+        assert!(config.clients["radar"].allowed_servers.is_empty());
+        assert!(config.clients["radar"].has_allow_list());
+    }
+
     #[test]
     fn a_tool_block_is_stored_by_server_and_tool_and_old_ones_are_brought_over() {
         use crate::config::ToolBlock;

plug-core/src/proxy/catalog.rs

@@ -846,7 +846,7 @@ impl super::ToolRouter {
     /// what belongs to a server it is kept from.
     pub fn list_resources_for_client(&self, client_key: Option<&str>) -> Arc<Vec<Resource>> {
         let snapshot = self.cache.load();
-        if !self.client_has_blocked_servers(client_key) {
+        if !self.client_is_kept_from_a_server(client_key) {
             return Arc::clone(&snapshot.resources_all);
         }
         Arc::new(
@@ -891,7 +891,7 @@ impl super::ToolRouter {
         client_key: Option<&str>,
     ) -> Arc<Vec<ResourceTemplate>> {
         let snapshot = self.cache.load();
-        if !self.client_has_blocked_servers(client_key) {
+        if !self.client_is_kept_from_a_server(client_key) {
             return Arc::clone(&snapshot.resource_templates_all);
         }
         Arc::new(
@@ -935,7 +935,7 @@ impl super::ToolRouter {
 
     pub fn list_prompts_for_client(&self, client_key: Option<&str>) -> Arc<Vec<Prompt>> {
         let snapshot = self.cache.load();
-        if !self.client_has_blocked_servers(client_key) {
+        if !self.client_is_kept_from_a_server(client_key) {
             return Arc::clone(&snapshot.prompts_all);
         }
         Arc::new(

plug-core/src/proxy/mod.rs

@@ -1193,7 +1193,8 @@ impl ToolRouter {
         }
     }
 
-    /// Whether the client behind `client_key` is kept from any server.
+    /// Whether the client behind `client_key` is kept from any server, so the
+    /// lists it is served need filtering at all.
     pub fn client_is_kept_from_a_server(&self, client_key: Option<&str>) -> bool {
         client_key.is_some_and(|key| {
             self.client_access
@@ -1220,6 +1221,11 @@ impl ToolRouter {
         let Some((server_id, own_name)) = cache.resolve_route(tool_name) else {
             return true;
         };
+        // Plug's own tools are how a client finds the rest. What they hand
+        // back is filtered for the client, so no list keeps them from it.
+        if server_id == "__plug_internal__" {
+            return true;
+        }
         !access.keeps_from(
             server_id,
             Some(own_name),
@@ -1280,9 +1286,12 @@ impl ToolRouter {
     /// the tool's result, so a block on the tool covers it.
     pub fn client_may_watch(&self, client_key: Option<&str>, server: &str, tool: &str) -> bool {
         self.client_may_use_server(client_key, server)
-            && self
-                .watched_tool(server, tool)
-                .is_none_or(|(name, _)| self.client_may_use_tool(client_key, &name))
+            && match self.watched_tool(server, tool) {
+                Some((name, _)) => self.client_may_use_tool(client_key, &name),
+                // Not listed now, so asked the way a kept result is: by
+                // server and the server's own name for the tool.
+                None => self.client_may_read_result_of(client_key, Some(server), Some(tool), ""),
+            }
     }
 
     /// Whether the client behind `client_key` may see `server_id` at all.
@@ -1320,17 +1329,6 @@ impl ToolRouter {
         }
     }
 
-    /// Whether the client behind `client_key` is kept from any server, so the
-    /// lists it is served need filtering at all.
-    pub(crate) fn client_has_blocked_servers(&self, client_key: Option<&str>) -> bool {
-        client_key.is_some_and(|client_key| {
-            self.client_access
-                .load()
-                .get(client_key)
-                .is_some_and(ClientAccess::keeps_from_a_server)
-        })
-    }
-
     fn ensure_client_may_use_tool(
         &self,
         downstream: Option<&DownstreamCallContext>,
@@ -2618,14 +2616,15 @@ impl ToolRouter {
 
             // A tool switched off stays off when a second account changes
             // its name, or when the account that changed it is removed.
-            if is_disabled_tool(&self.config.disabled_tools, &prefixed_name)
-                || crate::tool_naming::other_names(
-                    &prefixed_name,
-                    &c.server_name,
-                    &self.config.prefix_delimiter,
-                )
-                .iter()
-                .any(|name| is_disabled_tool(&self.config.disabled_tools, name))
+            if !self.config.disabled_tools.is_empty()
+                && (is_disabled_tool(&self.config.disabled_tools, &prefixed_name)
+                    || crate::tool_naming::other_names(
+                        &prefixed_name,
+                        &c.server_name,
+                        &self.config.prefix_delimiter,
+                    )
+                    .iter()
+                    .any(|name| is_disabled_tool(&self.config.disabled_tools, name)))
             {
                 continue;
             }

plug-core/src/proxy/tasks.rs

@@ -448,7 +448,14 @@ impl super::ToolRouter {
         // detaching the future, which kept running and kept holding its
         // server's `max_concurrent` semaphore permit.) No await is held
         // across this scope other than acquiring the lock itself.
-        let behind = self.tool_behind(tool_name);
+        // The way the call itself routes, so a tool called by its server's
+        // own name is recorded too.
+        let behind = self
+            .cache
+            .load()
+            .resolve_route(tool_name)
+            .filter(|(server, _)| server != "__plug_internal__")
+            .cloned();
         let created = {
             let mut store = self.task_store.lock().await;
             let task = match task_quota_lease {

plug-core/src/proxy/tests.rs

@@ -5108,6 +5108,32 @@ async fn the_meta_tools_do_not_reach_around_a_block() {
     );
 }
 
+#[test]
+fn an_allow_list_leaves_a_client_the_tool_it_searches_with() {
+    let mut config = test_router_config();
+    config.meta_tool_mode = true;
+    let router = router_with_client_blocks(config);
+    let mut clients = std::collections::BTreeMap::new();
+    clients.insert(
+        "radar".to_string(),
+        crate::config::ClientSettings {
+            allowed_servers: vec!["slack".to_string()],
+            ..Default::default()
+        },
+    );
+    router.set_client_access(&clients);
+
+    let names =
+        tool_names(&router.list_tools_for_client_session(ClientType::Unknown, None, Some("radar")));
+    assert!(
+        names.contains(&"plug__search_tools".to_string()),
+        "{names:?}"
+    );
+    assert!(router.client_may_use_tool(Some("radar"), "plug__search_tools"));
+    // What it finds is still only what is on the list.
+    assert!(!router.client_may_use_tool(Some("radar"), "git__commit"));
+}
+
 #[test]
 fn clients_hear_about_a_block_changing_and_about_nothing_else() {
     let router = ToolRouter::new(Arc::new(ServerManager::new()), test_router_config());

plug-core/src/session/stateful.rs

@@ -37,6 +37,13 @@ struct SessionState {
     broadcast_audience: crate::session::BroadcastAudience,
 }
 
+impl SessionState {
+    /// Whether the sign-in this session was opened under no longer holds.
+    fn access_ended(&self) -> bool {
+        self.still_allowed.as_ref().is_some_and(|check| !check())
+    }
+}
+
 impl StatefulSessionStore {
     /// Return a transport-aware snapshot of tracked HTTP sessions.
     pub fn list_sessions(&self) -> Vec<DownstreamSessionSnapshot> {
@@ -158,7 +165,7 @@ impl StatefulSessionStore {
             self.remove_if_expired(session_id);
             return Err(HttpError::SessionNotFound);
         }
-        if entry.still_allowed.as_ref().is_some_and(|check| !check()) {
+        if entry.access_ended() {
             drop(entry);
             self.end(session_id);
             return Err(HttpError::SessionNotFound);
@@ -192,7 +199,7 @@ impl StatefulSessionStore {
         if let Some(mut entry) = self.sessions.get_mut(session_id) {
             if entry.last_activity.elapsed() > self.timeout {
                 remove_session = true;
-            } else if entry.still_allowed.as_ref().is_some_and(|check| !check()) {
+            } else if entry.access_ended() {
                 drop(entry);
                 self.end(session_id);
                 return SessionSendOutcome::SessionNotFound;
@@ -540,7 +547,7 @@ impl SessionStore for StatefulSessionStore {
                 expired.push(session_id);
                 continue;
             }
-            if entry.still_allowed.as_ref().is_some_and(|check| !check()) {
+            if entry.access_ended() {
                 ended.push(session_id);
                 continue;
             }

plug/src/client_host.rs

@@ -37,7 +37,8 @@ pub(crate) fn current() -> Option<ClientHost> {
             if is_interpreter(executable) {
                 host.script = command_line(pid).as_deref().and_then(|line| {
                     let script = script_of(line)?;
-                    Some(if runs_a_module(line) {
+                    // Only a script typed without its folder needs it.
+                    Some(if runs_a_module(line) || script.starts_with('/') {
                         script
                     } else {
                         anchored(&script, working_folder(pid).as_deref())

plug/src/commands/clients.rs

@@ -169,7 +169,7 @@ pub(crate) enum ClientCommands {
         /// Take the named servers and tools off the list instead
         #[arg(long, conflicts_with = "off")]
         remove: bool,
-        /// End the list. The client keeps what it gets now, and gets
+        /// End the list. The client keeps the servers on it, and gets
         /// servers added later
         #[arg(long)]
         off: bool,