fix(skills): client-side filter for All-Sources hub search

Issue #79 — Browse Hub clearly listed "honcho" but searching for "honcho" with the source picker on "All Sources" returned nothing. Root cause is on the Hermes side: `hermes skills search <query>` without a `--source` flag routes through the centralized `hermes-index` source and skips the external API sources (skills-sh, github, clawhub, lobehub, well-known, claude-marketplace). Browse aggregates those sources too, so any skill that lives only in the API tier shows up in browse but disappears in search. Same picker, same query, contradictory results. Rather than chase Hermes's index gaps, redefine "All Sources" search in Scarf to mean filter-what-you-see — the canonical type-to-filter UX users already expect on a list. Source-specific searches keep the CLI shell-out for full upstream search semantics on that registry. Implementation: - New `lastBrowseResults` cache populated on every successful `browseHub()`. Setter is `internal` so the test suite can seed without invoking the live CLI; out-of-module callers can still only read. - `searchHub()` now branches on `hubSource`. The "all" branch filters the cache via `localizedCaseInsensitiveContains` against name, description, and identifier, runs synchronously on the calling actor (UI invocations are already on MainActor) so the user sees the narrowed list without a render-tick gap. - If the cache is empty (search-before-browse), `browseHubThenFilter` performs one CLI fetch, populates the cache, then applies the filter — failure surfaces a "Search failed" banner instead of a silent empty state. - Source-specific search still shells out to `hermes skills search <query> --source <s> --limit 40`. Adds five regression tests covering name match, description match, case-insensitive folding, no-match message state, and the empty-query fallthrough to browse. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 02:14:37 +00:00 · 2026-05-07 11:50:52 +02:00
parent a6a8cae8ff
commit f9e3cd38f5
2 changed files with 201 additions and 4 deletions
@@ -49,6 +49,18 @@ public final class SkillsViewModel {
    public var hubMessage: String?
    public var hubSource: String = "all"

+    /// Last successful `browseHub` payload, kept around so that the
+    /// "All Sources" search path can filter client-side (issue #79).
+    /// `hermes skills search` with no `--source` flag routes through
+    /// the centralized `hermes-index` source which can miss skills
+    /// that are visible in browse — we'd rather give the user the
+    /// canonical "type-to-filter" UX than chase Hermes's index gaps.
+    /// Source-specific searches still shell out to the CLI for full
+    /// upstream semantics. Setter is `internal` so the in-tree test
+    /// suite can seed the cache without invoking the live CLI;
+    /// out-of-module callers can still only read.
+    public internal(set) var lastBrowseResults: [HermesHubSkill] = []
+
    public let hubSources = ["all", "official", "skills-sh", "well-known", "github", "clawhub", "lobehub"]

    public var filteredCategories: [HermesSkillCategory] {
@@ -260,14 +272,34 @@ public final class SkillsViewModel {
            browseHub()
            return
        }
+        let source = hubSource
+        let query = hubQuery
+        // Issue #79 — for "All Sources", filter the cached browse list
+        // client-side instead of shelling out. Hermes's all-source
+        // search routes through its centralized index which can miss
+        // skills (e.g. honcho) that browse surfaces from non-indexed
+        // registries. Specific-source searches keep the CLI path so
+        // power users still get full upstream search semantics.
+        if source == "all" {
+            if lastBrowseResults.isEmpty {
+                // No cache yet — kick off a browse, then filter on
+                // completion. The chained call lets the user type a
+                // query before ever clicking Browse.
+                browseHubThenFilter(query: query)
+            } else {
+                // Pure in-memory filter — runs synchronously on the
+                // calling actor (UI invocations are already on
+                // MainActor) so the user sees the narrowed list
+                // without a render-tick gap.
+                applyClientSideFilter(query: query, against: lastBrowseResults)
+            }
+            return
+        }
        isHubLoading = true
        let bin = context.paths.hermesBinary
        let xport = transport
-        let source = hubSource
-        let query = hubQuery
        Task.detached { [weak self] in
-            var args = ["skills", "search", query, "--limit", "40"]
-            if source != "all" { args += ["--source", source] }
+            let args = ["skills", "search", query, "--limit", "40", "--source", source]
            let result = Self.runHermes(executable: bin, args: args, transport: xport, timeout: 30)
            let parsed = HermesSkillsHubParser.parseHubList(result.output)
            await self?.finishBrowse(
@@ -279,6 +311,66 @@ public final class SkillsViewModel {
        }
    }

+    /// Run a browse fetch and then immediately apply a client-side
+    /// filter. Used by `searchHub` when the user types into search
+    /// before any browse has cached results.
+    private func browseHubThenFilter(query: String) {
+        isHubLoading = true
+        let bin = context.paths.hermesBinary
+        let xport = transport
+        Task.detached { [weak self] in
+            let args = ["skills", "browse", "--size", "40"]
+            let result = Self.runHermes(executable: bin, args: args, transport: xport, timeout: 30)
+            let parsed = HermesSkillsHubParser.parseHubList(result.output)
+            await self?.finishBrowseThenFilter(
+                browseResults: parsed,
+                query: query,
+                exitCode: result.exitCode,
+                rawOutput: result.output
+            )
+        }
+    }
+
+    @MainActor
+    private func finishBrowseThenFilter(
+        browseResults: [HermesHubSkill],
+        query: String,
+        exitCode: Int32,
+        rawOutput: String
+    ) async {
+        if exitCode == 0 {
+            lastBrowseResults = browseResults
+            applyClientSideFilter(query: query, against: browseResults)
+        } else {
+            // Surface the underlying browse failure rather than a
+            // blank "no matches" state — the user typed a query, not
+            // a browse request, but the cache was empty so we tried.
+            isHubLoading = false
+            hubResults = []
+            let detail = Self.firstSignificantLine(rawOutput)
+            hubMessage = detail.isEmpty
+                ? "Search failed (exit \(exitCode))"
+                : "Search failed: \(detail)"
+        }
+    }
+
+    private func applyClientSideFilter(query: String, against pool: [HermesHubSkill]) {
+        let needle = query.trimmingCharacters(in: .whitespaces)
+        let matches: [HermesHubSkill]
+        if needle.isEmpty {
+            matches = pool
+        } else {
+            matches = pool.filter { skill in
+                skill.name.localizedCaseInsensitiveContains(needle)
+                    || skill.description.localizedCaseInsensitiveContains(needle)
+                    || skill.identifier.localizedCaseInsensitiveContains(needle)
+            }
+        }
+        isHubLoading = false
+        hubResults = matches
+        hubMessage = matches.isEmpty ? "No matches" : nil
+    }
+
    public func installHubSkill(_ skill: HermesHubSkill) {
        isHubLoading = true
        hubMessage = "Installing \(skill.identifier)…"
@@ -421,6 +513,13 @@ public final class SkillsViewModel {
    ) async {
        isHubLoading = false
        hubResults = results
+        // Cache the fresh browse payload so the "All Sources" search
+        // path can filter client-side (issue #79). Search results are
+        // not cached — they're already filtered by the user's query
+        // and would poison the filter pool.
+        if !isSearch && exitCode == 0 {
+            lastBrowseResults = results
+        }
        if results.isEmpty {
            if exitCode == 0 {
                hubMessage = isSearch ? "No matches" : "No results"
@@ -0,0 +1,98 @@
+import Testing
+import Foundation
+@testable import ScarfCore
+
+/// Issue #79 regression. `searchHub()` with `hubSource == "all"` must
+/// filter the cached browse list client-side (instead of shelling out
+/// to `hermes skills search`, which routes through Hermes's
+/// centralized index and can miss skills that browse aggregates from
+/// non-indexed registries — `honcho` was the user-reported example).
+///
+/// Source-specific searches keep the CLI path; that's not exercised
+/// here because it requires a live `hermes` binary — the existing
+/// HermesSkillsHubParser tests cover the parser side.
+@Suite("SkillsViewModel hub filter")
+@MainActor
+struct SkillsViewModelHubFilterTests {
+
+    private func makeViewModel() -> SkillsViewModel {
+        SkillsViewModel(context: .local)
+    }
+
+    private let stubBrowse: [HermesHubSkill] = [
+        HermesHubSkill(
+            identifier: "honcho",
+            name: "honcho",
+            description: "Memory provider for chat-scoped facts.",
+            source: "github"
+        ),
+        HermesHubSkill(
+            identifier: "1password",
+            name: "1password",
+            description: "Set up and use 1Password integration.",
+            source: "official"
+        ),
+        HermesHubSkill(
+            identifier: "spotify",
+            name: "spotify",
+            description: "Spotify skill — playback control via OAuth.",
+            source: "official"
+        ),
+    ]
+
+    @Test func allSourcesFilterMatchesByName() {
+        let vm = makeViewModel()
+        vm.lastBrowseResults = stubBrowse
+        vm.hubSource = "all"
+        vm.hubQuery = "honcho"
+        vm.searchHub()
+        #expect(vm.hubResults.count == 1)
+        #expect(vm.hubResults.first?.identifier == "honcho")
+        #expect(vm.isHubLoading == false)
+        #expect(vm.hubMessage == nil)
+    }
+
+    @Test func allSourcesFilterMatchesByDescription() {
+        let vm = makeViewModel()
+        vm.lastBrowseResults = stubBrowse
+        vm.hubSource = "all"
+        vm.hubQuery = "OAuth"
+        vm.searchHub()
+        #expect(vm.hubResults.count == 1)
+        #expect(vm.hubResults.first?.identifier == "spotify")
+    }
+
+    @Test func allSourcesFilterIsCaseInsensitive() {
+        let vm = makeViewModel()
+        vm.lastBrowseResults = stubBrowse
+        vm.hubSource = "all"
+        vm.hubQuery = "HONCHO"
+        vm.searchHub()
+        #expect(vm.hubResults.count == 1)
+        #expect(vm.hubResults.first?.identifier == "honcho")
+    }
+
+    @Test func allSourcesFilterEmptyMatchSetsMessage() {
+        let vm = makeViewModel()
+        vm.lastBrowseResults = stubBrowse
+        vm.hubSource = "all"
+        vm.hubQuery = "ringtone"
+        vm.searchHub()
+        #expect(vm.hubResults.isEmpty)
+        #expect(vm.hubMessage == "No matches")
+    }
+
+    /// Empty query should fall through to `browseHub()`, which on
+    /// `.local` with no Hermes installed will set isHubLoading=true
+    /// and not block the test. We just assert the early-return guard
+    /// kicked in by checking the cache was untouched.
+    @Test func emptyQueryFallsThroughToBrowse() {
+        let vm = makeViewModel()
+        vm.lastBrowseResults = stubBrowse
+        vm.hubSource = "all"
+        vm.hubQuery = ""
+        let cacheBefore = vm.lastBrowseResults
+        vm.searchHub()
+        #expect(vm.lastBrowseResults == cacheBefore)
+    }
+}