From c68d42f6a1c2048b8679953baac97621bf72fb21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Mon, 6 Jul 2026 20:51:43 +0800 Subject: [PATCH] fix: speed up skill marketplace detail previews Add server-side detail caching and in-flight request coalescing for marketplace details, reuse a long-lived service across API requests, and parallelize ClawHub detail preview fetches. Add desktop detail cache/prefetch behavior and render marketplace Markdown/code previews through the existing MarkdownRenderer and CodeViewer components. Stabilize the H5 diagnostics test on machines where the first non-internal IPv4 address is not a plain LAN address. Tested: bun test src/server/__tests__/skill-market.test.ts src/server/__tests__/skill-market-install.test.ts Tested: cd desktop && bun run test -- SkillCenter.test.tsx skillMarketStore.test.ts --run Tested: bun test src/server/__tests__/h5-access-service.test.ts -t 'getDiagnostics reports stale' Tested: git diff --check Tested: bun run check:desktop Tested: bun run check:server Not-tested: bun run verify and coverage gates; this is a local handoff, not PR-ready validation. Confidence: high Scope-risk: moderate --- desktop/src/pages/SkillCenter.test.tsx | 80 +++++++++-- desktop/src/pages/SkillCenter.tsx | 134 +++++++++++++----- desktop/src/stores/skillMarketStore.test.ts | 70 ++++++++- desktop/src/stores/skillMarketStore.ts | 106 ++++++++++++-- .../__tests__/h5-access-service.test.ts | 3 +- src/server/__tests__/skill-market.test.ts | 102 ++++++++++++- src/server/api/skill-market.ts | 28 ++-- src/server/services/skillMarket/service.ts | 128 +++++++++++++---- 8 files changed, 562 insertions(+), 89 deletions(-) diff --git a/desktop/src/pages/SkillCenter.test.tsx b/desktop/src/pages/SkillCenter.test.tsx index 6ae17f37..a0730138 100644 --- a/desktop/src/pages/SkillCenter.test.tsx +++ b/desktop/src/pages/SkillCenter.test.tsx @@ -21,13 +21,35 @@ vi.mock('../components/skills/SkillList', () => ({ })) vi.mock('../components/markdown/MarkdownRenderer', () => ({ - MarkdownRenderer: ({ content }: { content: string }) => ( -
{content}
+ MarkdownRenderer: ({ content, variant }: { content: string; variant?: string }) => ( +
{content}
), })) vi.mock('../components/chat/CodeViewer', () => ({ - CodeViewer: ({ code }: { code: string }) =>
{code}
, + CodeViewer: ({ + code, + language, + maxLines, + showLineNumbers, + wrapLongLines, + }: { + code: string + language?: string + maxLines?: number + showLineNumbers?: boolean + wrapLongLines?: boolean + }) => ( +
+ {code} +
+ ), })) const mockedSkillMarketApi = vi.mocked(skillMarketApi) @@ -74,10 +96,12 @@ function makeDetail(overrides: Partial = {}): SkillMarketDeta describe('SkillCenter', () => { beforeEach(() => { useSettingsStore.setState({ locale: 'en' }) + useSkillMarketStore.getState().clearDetailCache() useSkillMarketStore.setState({ items: [], nextCursor: null, selectedDetail: null, + detailCache: {}, source: 'auto', resolvedSource: null, sourceStatus: null, @@ -87,6 +111,7 @@ describe('SkillCenter', () => { isLoading: false, isLoadingMore: false, isDetailLoading: false, + loadingDetailKey: null, isInstalling: false, error: null, }) @@ -119,10 +144,12 @@ describe('SkillCenter', () => { cleanup() vi.clearAllMocks() useSettingsStore.setState({ locale: 'en' }) + useSkillMarketStore.getState().clearDetailCache() useSkillMarketStore.setState({ items: [], nextCursor: null, selectedDetail: null, + detailCache: {}, source: 'auto', resolvedSource: null, sourceStatus: null, @@ -132,6 +159,7 @@ describe('SkillCenter', () => { isLoading: false, isLoadingMore: false, isDetailLoading: false, + loadingDetailKey: null, isInstalling: false, error: null, }) @@ -172,7 +200,7 @@ describe('SkillCenter', () => { expect(screen.getByRole('button', { name: 'Install' })).toBeEnabled() expect(screen.getByText('Open upstream')).toHaveAttribute('href', 'https://github.com/example/ppt-generator') expect(screen.getByText('File preview')).toBeInTheDocument() - expect(screen.getByText('SKILL.md')).toBeInTheDocument() + expect(screen.getAllByText('SKILL.md').length).toBeGreaterThan(0) fireEvent.click(screen.getByRole('button', { name: 'Install' })) expect(mockedSkillMarketApi.install).not.toHaveBeenCalled() @@ -234,6 +262,35 @@ describe('SkillCenter', () => { expect(within(vetterCard).getByText(/260\.9K|260,960/)).toBeInTheDocument() }) + it('prefetches visible details and warms a focused marketplace card without opening the drawer', async () => { + mockedSkillMarketApi.list.mockResolvedValue({ + items: [ + makeItem({ slug: 'alpha', displayName: 'Alpha Skill' }), + makeItem({ slug: 'bravo', displayName: 'Bravo Skill' }), + makeItem({ slug: 'charlie', displayName: 'Charlie Skill' }), + makeItem({ slug: 'delta', displayName: 'Delta Skill' }), + ], + nextCursor: null, + source: 'clawhub', + sourceStatus: 'ok', + }) + + render() + + const deltaCard = await screen.findByRole('button', { name: 'Delta Skill' }) + await waitFor(() => { + expect(mockedSkillMarketApi.detail).toHaveBeenCalledTimes(3) + }) + mockedSkillMarketApi.detail.mockClear() + + fireEvent.focus(deltaCard) + + await waitFor(() => { + expect(mockedSkillMarketApi.detail).toHaveBeenCalledWith('clawhub', 'delta') + }) + expect(screen.queryByTestId('skill-market-detail-layer')).not.toBeInTheDocument() + }) + it('uses marketplace-shaped loading skeletons while the catalog loads', () => { mockedSkillMarketApi.list.mockReturnValue(new Promise(() => {})) act(() => { @@ -311,10 +368,10 @@ describe('SkillCenter', () => { expect(within(dialog).getByRole('button', { name: 'Blocked' })).toBeDisabled() expect(within(dialog).getByText(/full package safety scan/i)).toBeInTheDocument() expect(within(dialog).getByText('File preview')).toBeInTheDocument() - expect(within(dialog).getByText('SKILL.md')).toBeInTheDocument() + expect(within(dialog).getAllByText('SKILL.md').length).toBeGreaterThan(0) }) - it('renders raw Markdown and Python previews for an uninstalled marketplace skill', async () => { + it('renders Markdown and Python previews with the existing rich preview components', async () => { mockedSkillMarketApi.detail.mockResolvedValue({ detail: makeDetail({ files: [ @@ -344,9 +401,16 @@ describe('SkillCenter', () => { fireEvent.click(await screen.findByRole('button', { name: 'PPT Generator' })) expect(await screen.findByText('File preview')).toBeInTheDocument() + expect(screen.getByTestId('markdown-renderer')).toHaveAttribute('data-variant', 'compact') + expect(screen.getByTestId('markdown-renderer')).toHaveTextContent('Use this before install.') expect(screen.getByText('scripts/audit.py')).toBeInTheDocument() - expect(screen.getByText('python')).toBeInTheDocument() - expect(screen.getByText(/print\("audit"\)/)).toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: 'scripts/audit.py' })) + const codePreview = screen.getByTestId('code-viewer') + expect(codePreview).toHaveAttribute('data-language', 'python') + expect(codePreview).toHaveAttribute('data-max-lines', '9999') + expect(codePreview).toHaveAttribute('data-show-line-numbers', 'true') + expect(codePreview).toHaveAttribute('data-wrap-long-lines', 'true') + expect(codePreview).toHaveTextContent('print("audit")') expect(screen.getByText('Scripts')).toBeInTheDocument() expect(screen.getByText('Executables')).toBeInTheDocument() }) diff --git a/desktop/src/pages/SkillCenter.tsx b/desktop/src/pages/SkillCenter.tsx index 40c67850..6b9fd855 100644 --- a/desktop/src/pages/SkillCenter.tsx +++ b/desktop/src/pages/SkillCenter.tsx @@ -22,14 +22,17 @@ import { } from 'lucide-react' import { SkillList } from '../components/skills/SkillList' import { SkillDetail } from '../components/skills/SkillDetail' +import { MarkdownRenderer } from '../components/markdown/MarkdownRenderer' +import { CodeViewer } from '../components/chat/CodeViewer' import { ConfirmDialog } from '../components/shared/ConfirmDialog' import { useTranslation } from '../i18n' import { formatBytes } from '../lib/formatBytes' import { useSessionStore } from '../stores/sessionStore' -import { useSkillMarketStore } from '../stores/skillMarketStore' +import { skillMarketDetailKey, useSkillMarketStore } from '../stores/skillMarketStore' import { useSkillStore } from '../stores/skillStore' import type { SkillMarketDetail, + SkillMarketFilePreview, SkillMarketInstallEligibility, SkillMarketItem, SkillMarketListSource, @@ -46,6 +49,7 @@ const SOURCE_OPTIONS: SkillMarketListSource[] = ['auto', 'clawhub', 'skillhub'] const SORT_OPTIONS: SkillMarketSort[] = ['downloads', 'installs', 'stars', 'updated', 'trending'] const FILTER_OPTIONS: MarketFilter[] = ['all', 'safe', 'popular', 'installed', 'apiKey'] const TRUST_SAFE: SkillMarketTrustState[] = ['clean', 'benign', 'signed', 'official'] +const VISIBLE_DETAIL_PREFETCH_LIMIT = 3 export function SkillCenter() { const t = useTranslation() @@ -65,6 +69,7 @@ export function SkillCenter() { items, nextCursor, selectedDetail, + loadingDetailKey, source, resolvedSource, sourceStatus, @@ -82,6 +87,7 @@ export function SkillCenter() { fetchItems, fetchMore, fetchDetail, + prefetchDetail, installSelected, clearDetail, } = useSkillMarketStore() @@ -109,6 +115,17 @@ export function SkillCenter() { () => items.filter((item) => matchesMarketFilter(item, marketFilter)), [items, marketFilter], ) + const activeMarketDetailKey = selectedDetail + ? skillMarketDetailKey(selectedDetail.source, selectedDetail.slug) + : loadingDetailKey + + useEffect(() => { + if (activeTab !== 'marketplace' || isLoading || filteredItems.length === 0) return + + for (const item of filteredItems.slice(0, VISIBLE_DETAIL_PREFETCH_LIMIT)) { + void prefetchDetail(item.source, item.slug) + } + }, [activeTab, filteredItems, isLoading, prefetchDetail]) const handleSearch = (event?: FormEvent) => { event?.preventDefault() @@ -313,8 +330,9 @@ export function SkillCenter() { void fetchDetail(item.source, item.slug)} + onPrefetch={() => void prefetchDetail(item.source, item.slug)} /> ))} @@ -573,10 +591,12 @@ function SkillMarketCard({ item, active, onSelect, + onPrefetch, }: { item: SkillMarketItem active: boolean onSelect: () => void + onPrefetch: () => void }) { const t = useTranslation() const stats = formatStats(item, t) @@ -588,6 +608,8 @@ function SkillMarketCard({ type="button" aria-label={item.displayName} onClick={onSelect} + onPointerEnter={onPrefetch} + onFocus={onPrefetch} className={[ 'group flex min-h-[124px] flex-col rounded-lg border bg-[var(--color-surface)] p-3 text-left transition-[border-color,background-color,box-shadow,transform] duration-200 ease-out active:translate-y-px', 'hover:border-[var(--color-border-focus)] hover:bg-[var(--color-surface-hover)] hover:shadow-sm', @@ -911,6 +933,7 @@ function SkillMarketDetailPanel({ function FilePreviewSection({ detail }: { detail: SkillMarketDetail }) { const t = useTranslation() + const [selectedPath, setSelectedPath] = useState(null) const previews = detail.filePreviews?.length ? detail.filePreviews : detail.entryPreview @@ -920,6 +943,7 @@ function FilePreviewSection({ detail }: { detail: SkillMarketDetail }) { language: 'markdown', }] : [] + const activePreview = previews.find((preview) => preview.path === selectedPath) ?? previews[0] if (previews.length === 0) { if (!detail.previewUnavailableReason) { @@ -938,44 +962,86 @@ function FilePreviewSection({ detail }: { detail: SkillMarketDetail }) { ) } + if (!activePreview) { + return null + } + return ( -
-
- {previews.map((preview) => ( -
-
- - {preview.path} - - {preview.language ? ( - - {preview.language} - - ) : null} - {typeof preview.size === 'number' ? ( - - {formatBytes(preview.size)} - - ) : null} - {preview.truncated ? ( - - {t('skillCenter.marketplace.previewTruncated')} - - ) : null} -
- 
-              {preview.content}
-
-
- ))} +
+
+ {previews.map((preview) => { + const active = preview.path === activePreview.path + return ( + + ) + })} +
+
+ + {activePreview.path} + + {activePreview.language ? ( + + {normalizePreviewLanguage(activePreview.language)} + + ) : null} + {typeof activePreview.size === 'number' ? ( + + {formatBytes(activePreview.size)} + + ) : null} + {activePreview.truncated ? ( + + {t('skillCenter.marketplace.previewTruncated')} + + ) : null} +
+
+ {isMarkdownPreview(activePreview) ? ( + + ) : ( +
+ +
+ )}
) } +function isMarkdownPreview(preview: SkillMarketFilePreview): boolean { + const language = normalizePreviewLanguage(preview.language) + const path = preview.path.toLowerCase() + return language === 'markdown' || path.endsWith('.md') || path.endsWith('.mdx') +} + +function normalizePreviewLanguage(language: string | undefined): string | undefined { + if (language === 'shell') return 'bash' + return language +} + function DrawerSection({ title, children }: { title: string; children: ReactNode }) { return (
diff --git a/desktop/src/stores/skillMarketStore.test.ts b/desktop/src/stores/skillMarketStore.test.ts index 90f35ffc..b9b2b184 100644 --- a/desktop/src/stores/skillMarketStore.test.ts +++ b/desktop/src/stores/skillMarketStore.test.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { skillMarketApi } from '../api/skillMarket' import type { SkillMarketDetail, SkillMarketItem } from '../types/skillMarket' -import { useSkillMarketStore } from './skillMarketStore' +import { skillMarketDetailKey, useSkillMarketStore } from './skillMarketStore' vi.mock('../api/skillMarket', () => ({ skillMarketApi: { @@ -40,10 +40,12 @@ function makeDetail(overrides: Partial = {}): SkillMarketDeta describe('skillMarketStore', () => { beforeEach(() => { vi.clearAllMocks() + useSkillMarketStore.getState().clearDetailCache() useSkillMarketStore.setState({ items: [], nextCursor: null, selectedDetail: null, + detailCache: {}, source: 'auto', resolvedSource: null, sourceStatus: null, @@ -53,6 +55,7 @@ describe('skillMarketStore', () => { isLoading: false, isLoadingMore: false, isDetailLoading: false, + loadingDetailKey: null, isInstalling: false, error: null, }) @@ -197,6 +200,71 @@ describe('skillMarketStore', () => { expect(useSkillMarketStore.getState().error).toBeNull() }) + it('prefetches detail into cache without opening the drawer', async () => { + const detail = makeDetail() + mockedSkillMarketApi.detail.mockResolvedValue({ detail }) + + await useSkillMarketStore.getState().prefetchDetail('clawhub', 'skill-vetter') + + expect(mockedSkillMarketApi.detail).toHaveBeenCalledWith('clawhub', 'skill-vetter') + expect(useSkillMarketStore.getState().selectedDetail).toBeNull() + expect(useSkillMarketStore.getState().isDetailLoading).toBe(false) + expect(useSkillMarketStore.getState().detailCache[skillMarketDetailKey('clawhub', 'skill-vetter')]).toEqual(detail) + }) + + it('deduplicates repeated detail prefetch requests', async () => { + let resolveDetail: ((value: { detail: SkillMarketDetail }) => void) | undefined + const detail = makeDetail() + mockedSkillMarketApi.detail.mockReturnValue(new Promise((resolve) => { + resolveDetail = resolve + })) + + const first = useSkillMarketStore.getState().prefetchDetail('clawhub', 'skill-vetter') + const second = useSkillMarketStore.getState().prefetchDetail('clawhub', 'skill-vetter') + expect(mockedSkillMarketApi.detail).toHaveBeenCalledTimes(1) + + resolveDetail?.({ detail }) + await Promise.all([first, second]) + + expect(useSkillMarketStore.getState().detailCache[skillMarketDetailKey('clawhub', 'skill-vetter')]).toEqual(detail) + }) + + it('opens cached detail without another detail request', async () => { + const detail = makeDetail() + useSkillMarketStore.setState({ + detailCache: { + [skillMarketDetailKey('clawhub', 'skill-vetter')]: detail, + }, + }) + + await useSkillMarketStore.getState().fetchDetail('clawhub', 'skill-vetter') + + expect(mockedSkillMarketApi.detail).not.toHaveBeenCalled() + expect(useSkillMarketStore.getState().selectedDetail).toEqual(detail) + expect(useSkillMarketStore.getState().isDetailLoading).toBe(false) + expect(useSkillMarketStore.getState().loadingDetailKey).toBeNull() + }) + + it('reuses an in-flight prefetch when opening detail', async () => { + let resolveDetail: ((value: { detail: SkillMarketDetail }) => void) | undefined + const detail = makeDetail() + mockedSkillMarketApi.detail.mockReturnValue(new Promise((resolve) => { + resolveDetail = resolve + })) + + const prefetch = useSkillMarketStore.getState().prefetchDetail('clawhub', 'skill-vetter') + const open = useSkillMarketStore.getState().fetchDetail('clawhub', 'skill-vetter') + expect(mockedSkillMarketApi.detail).toHaveBeenCalledTimes(1) + expect(useSkillMarketStore.getState().loadingDetailKey).toBe(skillMarketDetailKey('clawhub', 'skill-vetter')) + + resolveDetail?.({ detail }) + await Promise.all([prefetch, open]) + + expect(useSkillMarketStore.getState().selectedDetail).toEqual(detail) + expect(useSkillMarketStore.getState().isDetailLoading).toBe(false) + expect(useSkillMarketStore.getState().loadingDetailKey).toBeNull() + }) + it('ignores stale marketplace detail responses', async () => { let resolveFirst: ((value: { detail: SkillMarketDetail }) => void) | undefined let resolveSecond: ((value: { detail: SkillMarketDetail }) => void) | undefined diff --git a/desktop/src/stores/skillMarketStore.ts b/desktop/src/stores/skillMarketStore.ts index 344aeb91..f272387d 100644 --- a/desktop/src/stores/skillMarketStore.ts +++ b/desktop/src/stores/skillMarketStore.ts @@ -13,6 +13,7 @@ type SkillMarketStore = { items: SkillMarketItem[] nextCursor: string | null selectedDetail: SkillMarketDetail | null + detailCache: Record source: SkillMarketListSource resolvedSource: SkillMarketSource | null sourceStatus: SkillMarketListResponse['sourceStatus'] | null @@ -22,6 +23,7 @@ type SkillMarketStore = { isLoading: boolean isLoadingMore: boolean isDetailLoading: boolean + loadingDetailKey: string | null isInstalling: boolean error: string | null setSource: (source: SkillMarketListSource) => void @@ -29,18 +31,27 @@ type SkillMarketStore = { setQuery: (query: string) => void fetchItems: (options?: { query?: string }) => Promise fetchMore: () => Promise - fetchDetail: (source: SkillMarketSource, slug: string) => Promise + fetchDetail: (source: SkillMarketSource, slug: string, options?: { force?: boolean }) => Promise + prefetchDetail: (source: SkillMarketSource, slug: string) => Promise installSelected: () => Promise clearDetail: () => void + clearDetailCache: () => void } let detailRequestSeq = 0 +const detailInFlightRequests = new Map>() const MARKET_PAGE_LIMIT = 100 +const DETAIL_CACHE_MAX_ENTRIES = 50 + +export function skillMarketDetailKey(source: SkillMarketSource, slug: string): string { + return `${source}:${slug}` +} export const useSkillMarketStore = create((set, get) => ({ items: [], nextCursor: null, selectedDetail: null, + detailCache: {}, source: 'auto', resolvedSource: null, sourceStatus: null, @@ -50,16 +61,17 @@ export const useSkillMarketStore = create((set, get) => ({ isLoading: false, isLoadingMore: false, isDetailLoading: false, + loadingDetailKey: null, isInstalling: false, error: null, setSource: (source) => { detailRequestSeq += 1 - set({ source, selectedDetail: null, nextCursor: null, isDetailLoading: false, statusMessage: null }) + set({ source, selectedDetail: null, nextCursor: null, isDetailLoading: false, loadingDetailKey: null, statusMessage: null }) }, setSort: (sort) => { detailRequestSeq += 1 - set({ sort, selectedDetail: null, nextCursor: null, isDetailLoading: false, statusMessage: null }) + set({ sort, selectedDetail: null, nextCursor: null, isDetailLoading: false, loadingDetailKey: null, statusMessage: null }) }, setQuery: (query) => set({ query }), @@ -67,7 +79,7 @@ export const useSkillMarketStore = create((set, get) => ({ const { source, sort, query } = get() const requestedQuery = options?.query ?? query detailRequestSeq += 1 - set({ isLoading: true, isLoadingMore: false, isDetailLoading: false, selectedDetail: null, error: null }) + set({ isLoading: true, isLoadingMore: false, isDetailLoading: false, loadingDetailKey: null, selectedDetail: null, error: null }) try { const result = await skillMarketApi.list({ source, @@ -121,23 +133,53 @@ export const useSkillMarketStore = create((set, get) => ({ } }, - fetchDetail: async (source, slug) => { + fetchDetail: async (source, slug, options) => { + const cacheKey = skillMarketDetailKey(source, slug) const requestId = detailRequestSeq + 1 detailRequestSeq = requestId - set({ isDetailLoading: true, selectedDetail: null, error: null }) + const cachedDetail = options?.force ? undefined : get().detailCache[cacheKey] + if (cachedDetail) { + set({ + selectedDetail: cachedDetail, + isDetailLoading: false, + loadingDetailKey: null, + error: null, + }) + return + } + + set({ isDetailLoading: true, loadingDetailKey: cacheKey, selectedDetail: null, error: null }) try { - const { detail } = await skillMarketApi.detail(source, slug) + const detail = await loadDetail(source, slug, options) + set((state) => ({ + detailCache: cacheDetail(state.detailCache, cacheKey, detail), + })) if (requestId !== detailRequestSeq) return - set({ selectedDetail: detail, isDetailLoading: false }) + set({ selectedDetail: detail, isDetailLoading: false, loadingDetailKey: null }) } catch (err) { if (requestId !== detailRequestSeq) return set({ isDetailLoading: false, + loadingDetailKey: null, error: getErrorMessage(err), }) } }, + prefetchDetail: async (source, slug) => { + const cacheKey = skillMarketDetailKey(source, slug) + if (get().detailCache[cacheKey]) return + + try { + const detail = await loadDetail(source, slug) + set((state) => ({ + detailCache: cacheDetail(state.detailCache, cacheKey, detail), + })) + } catch { + // Prefetch is opportunistic; the explicit detail open will report failures. + } + }, + installSelected: async () => { const detail = get().selectedDetail if (!detail) return @@ -146,7 +188,7 @@ export const useSkillMarketStore = create((set, get) => ({ try { await skillMarketApi.install(detail.source, detail.slug, detail.version) await get().fetchItems() - await get().fetchDetail(detail.source, detail.slug) + await get().fetchDetail(detail.source, detail.slug, { force: true }) set({ isInstalling: false }) } catch (err) { set({ @@ -158,10 +200,54 @@ export const useSkillMarketStore = create((set, get) => ({ clearDetail: () => { detailRequestSeq += 1 - set({ selectedDetail: null, isDetailLoading: false }) + set({ selectedDetail: null, isDetailLoading: false, loadingDetailKey: null }) + }, + + clearDetailCache: () => { + detailInFlightRequests.clear() + set({ detailCache: {} }) }, })) +function loadDetail( + source: SkillMarketSource, + slug: string, + options?: { force?: boolean }, +): Promise { + const cacheKey = skillMarketDetailKey(source, slug) + if (!options?.force) { + const pending = detailInFlightRequests.get(cacheKey) + if (pending) return pending + } + + const request = skillMarketApi.detail(source, slug) + .then(({ detail }) => detail) + .finally(() => { + if (detailInFlightRequests.get(cacheKey) === request) { + detailInFlightRequests.delete(cacheKey) + } + }) + + detailInFlightRequests.set(cacheKey, request) + return request +} + +function cacheDetail( + current: Record, + key: string, + detail: SkillMarketDetail, +): Record { + const next = { ...current, [key]: detail } + const keys = Object.keys(next) + while (keys.length > DETAIL_CACHE_MAX_ENTRIES) { + const oldestKey = keys.shift() + if (oldestKey) { + delete next[oldestKey] + } + } + return next +} + function mergeMarketItems(current: SkillMarketItem[], incoming: SkillMarketItem[]): SkillMarketItem[] { const seen = new Set(current.map((item) => `${item.source}:${item.slug}`)) const merged = [...current] diff --git a/src/server/__tests__/h5-access-service.test.ts b/src/server/__tests__/h5-access-service.test.ts index f564f0c6..a7073c52 100644 --- a/src/server/__tests__/h5-access-service.test.ts +++ b/src/server/__tests__/h5-access-service.test.ts @@ -581,7 +581,8 @@ describe('H5AccessService', () => { expect(diag.storedPublicBaseUrl).toBe('https://h5.mydomain.com') // ok: stored URL host is on local interfaces - const localHost = collectLocalIPv4Hosts()[0] + const localHost = collectLocalIPv4Hosts() + .find((host) => classifyH5PublicBaseUrl(`http://${host}:55379`) === 'plain-lan') if (localHost) { await service.updateSettings({ publicBaseUrl: `http://${localHost}:55379` }) diag = await service.getDiagnostics() diff --git a/src/server/__tests__/skill-market.test.ts b/src/server/__tests__/skill-market.test.ts index 67b2075b..2850f258 100644 --- a/src/server/__tests__/skill-market.test.ts +++ b/src/server/__tests__/skill-market.test.ts @@ -841,6 +841,87 @@ describe('skill market service source selection', () => { }) }) + it('caches remote detail previews without caching installed eligibility', async () => { + const fetchCalls: string[] = [] + let installedSkillNames = new Set() + const service = createSkillMarketService({ + fetchImpl: async (url) => { + const urlString = String(url) + fetchCalls.push(urlString) + if (urlString.endsWith('/scan')) { + return Response.json(CLAWHUB_NESTED_SCAN_RESPONSE) + } + if (urlString.endsWith('/versions/1.0.0')) { + return Response.json(CLAWHUB_VERSION_RESPONSE) + } + if (urlString.includes('/file?path=SKILL.md')) { + return new Response('# Skill Vetter\n\nallowed-tools: Bash\n') + } + if (urlString.includes('/file?path=scripts%2Faudit.py') || urlString.includes('/file?path=scripts/audit.py')) { + return new Response('print("audit")\n') + } + return Response.json(CLAWHUB_DETAIL_RESPONSE) + }, + installedSkillNames: async () => installedSkillNames, + now: () => 50_000, + }) + + const first = await service.getDetail({ source: 'clawhub', slug: 'skill-vetter' }) + installedSkillNames = new Set(['skill-vetter']) + const second = await service.getDetail({ source: 'clawhub', slug: 'skill-vetter' }) + + expect(fetchCalls).toHaveLength(5) + expect(first).toMatchObject({ + installed: false, + installEligibility: { status: 'installable' }, + filePreviews: [ + { path: 'SKILL.md', content: expect.stringContaining('# Skill Vetter') }, + { path: 'scripts/audit.py', content: expect.stringContaining('print("audit")') }, + ], + }) + expect(second).toMatchObject({ + installed: true, + installEligibility: { + status: 'installed', + installedSkillName: 'skill-vetter', + }, + }) + }) + + it('coalesces concurrent detail requests for the same skill', async () => { + const fetchCalls: string[] = [] + const service = createSkillMarketService({ + fetchImpl: async (url) => { + const urlString = String(url) + fetchCalls.push(urlString) + if (urlString.endsWith('/scan')) { + return Response.json(CLAWHUB_NESTED_SCAN_RESPONSE) + } + if (urlString.endsWith('/versions/1.0.0')) { + return Response.json(CLAWHUB_VERSION_RESPONSE) + } + if (urlString.includes('/file?path=SKILL.md')) { + return new Response('# Skill Vetter\n') + } + if (urlString.includes('/file?path=scripts%2Faudit.py') || urlString.includes('/file?path=scripts/audit.py')) { + return new Response('print("audit")\n') + } + return Response.json(CLAWHUB_DETAIL_RESPONSE) + }, + installedSkillNames: async () => new Set(), + now: () => 60_000, + }) + + const [first, second] = await Promise.all([ + service.getDetail({ source: 'clawhub', slug: 'skill-vetter' }), + service.getDetail({ source: 'clawhub', slug: 'skill-vetter' }), + ]) + + expect(fetchCalls).toHaveLength(5) + expect(first?.slug).toBe('skill-vetter') + expect(second?.slug).toBe('skill-vetter') + }) + it('blocks uninstalled detail installs when only list-level ClawHub metadata is available', async () => { const fetchCalls: string[] = [] const service = createSkillMarketService({ @@ -857,7 +938,8 @@ describe('skill market service source selection', () => { const detail = await service.getDetail({ source: 'clawhub', slug: 'skill-vetter' }) expect(fetchCalls[0]).toBe('https://clawhub.ai/api/v1/skills/skill-vetter') - expect(fetchCalls[1]).toContain('https://clawhub.ai/api/v1/skills?') + expect(fetchCalls[1]).toBe('https://clawhub.ai/api/v1/skills/skill-vetter/scan') + expect(fetchCalls[2]).toContain('https://clawhub.ai/api/v1/skills?') expect(detail).toMatchObject({ source: 'clawhub', slug: 'skill-vetter', @@ -1022,6 +1104,24 @@ describe('skill market API', () => { expect(res.status).toBe(405) }) + it('reuses the default service cache across list API requests', async () => { + const fetchCalls: string[] = [] + globalThis.fetch = (async (url) => { + fetchCalls.push(String(url)) + return Response.json(CLAWHUB_TOP_SKILLS_RESPONSE) + }) as typeof fetch + const url = new URL('/api/skill-market?source=clawhub&limit=12&q=vetter', 'http://localhost:3456') + + const first = await handleSkillMarketApi(new Request(url, { method: 'GET' }), url, ['api', 'skill-market']) + const second = await handleSkillMarketApi(new Request(url, { method: 'GET' }), url, ['api', 'skill-market']) + + expect(first.status).toBe(200) + expect(second.status).toBe(200) + expect(fetchCalls).toHaveLength(1) + await expect(first.json()).resolves.toMatchObject({ sourceStatus: 'ok' }) + await expect(second.json()).resolves.toMatchObject({ sourceStatus: 'cached' }) + }) + it('rejects install requests with target paths', async () => { const url = new URL('/api/skill-market/install', 'http://localhost:3456') const req = new Request(url, { diff --git a/src/server/api/skill-market.ts b/src/server/api/skill-market.ts index 538ff9db..49bbc90a 100644 --- a/src/server/api/skill-market.ts +++ b/src/server/api/skill-market.ts @@ -34,13 +34,16 @@ const PACKAGE_URL_FIELDS = [ ] as const let skillMarketServiceFactory: SkillMarketServiceFactory = createSkillMarketService +let defaultSkillMarketService: SkillMarketService | null = null export function setSkillMarketServiceFactoryForTests(factory: SkillMarketServiceFactory): void { skillMarketServiceFactory = factory + defaultSkillMarketService = null } export function resetSkillMarketServiceFactoryForTests(): void { skillMarketServiceFactory = createSkillMarketService + defaultSkillMarketService = null } export async function handleSkillMarketApi( @@ -56,9 +59,7 @@ export async function handleSkillMarketApi( return params } - const service = skillMarketServiceFactory({ - installedSkillNames: collectUserSkillNames, - }) + const service = getSkillMarketService() const result = await service.list(params) return Response.json(result) } @@ -91,9 +92,7 @@ async function handleDetail(req: Request, segments: string[], sourceSegment: str return jsonError('not_found', 'Skill market skill not found.', 404) } - const service = skillMarketServiceFactory({ - installedSkillNames: collectUserSkillNames, - }) + const service = getSkillMarketService() const detail = await service.getDetail({ source: sourceSegment as SkillMarketSource, slug, @@ -175,9 +174,7 @@ async function handleInstall(req: Request): Promise { return installRequest } - const service = skillMarketServiceFactory({ - installedSkillNames: collectUserSkillNames, - }) + const service = getSkillMarketService() const detail = await service.getDetail({ source: installRequest.source, slug: installRequest.slug, @@ -244,6 +241,19 @@ async function handleInstall(req: Request): Promise { } } +function getSkillMarketService(): SkillMarketService { + if (skillMarketServiceFactory !== createSkillMarketService) { + return skillMarketServiceFactory({ + installedSkillNames: collectUserSkillNames, + }) + } + + defaultSkillMarketService ??= skillMarketServiceFactory({ + installedSkillNames: collectUserSkillNames, + }) + return defaultSkillMarketService +} + function parseInstallRequest(body: Record): SkillMarketInstallRequest | Response { for (const key of Object.keys(body)) { if (!INSTALL_REQUEST_KEYS.has(key)) { diff --git a/src/server/services/skillMarket/service.ts b/src/server/services/skillMarket/service.ts index 7954f661..2582b588 100644 --- a/src/server/services/skillMarket/service.ts +++ b/src/server/services/skillMarket/service.ts @@ -45,6 +45,7 @@ const SKILLHUB_SKILLS_URL = 'https://api.skillhub.cn/api/skills' const DEFAULT_LIMIT = 100 const MAX_LIMIT = 100 const CATALOG_CACHE_TTL_MS = 5 * 60 * 1_000 +const DETAIL_CACHE_TTL_MS = 10 * 60 * 1_000 const FAILURE_CACHE_TTL_MS = 60 * 1_000 const FILE_PREVIEW_LIMIT = 5 const FILE_PREVIEW_MAX_BYTES = 96 * 1024 @@ -82,6 +83,11 @@ type CatalogCacheEntry = { result: SkillMarketListResult } +type DetailCacheEntry = { + expiresAt: number + detail: SkillMarketDetail | null +} + type FailureCacheEntry = { expiresAt: number message: string @@ -100,6 +106,8 @@ export function createSkillMarketService(options: SkillMarketServiceOptions = {} const installedSkillNames = options.installedSkillNames const now = options.now ?? Date.now const catalogCache = new Map() + const detailCache = new Map() + const pendingDetailRequests = new Map>() const failureCache = new Map() async function listSkills(params: SkillMarketListParams = {}): Promise { @@ -233,33 +241,79 @@ export function createSkillMarketService(options: SkillMarketServiceOptions = {} return null } - if (params.source === 'clawhub') { - const installed = await resolveInstalledSkillNames(installedSkillNames) + const installed = await resolveInstalledSkillNames(installedSkillNames) + const baseDetail = await cachedDetail(detailCacheKey(params.source, slug), () => fetchBaseDetail(params.source, slug)) + return baseDetail ? applyInstalledState(baseDetail, installed.has(slug)) : null + } + + async function cachedDetail( + cacheKey: string, + loader: () => Promise, + ): Promise { + const cached = detailCache.get(cacheKey) + const currentTime = now() + if (cached && cached.expiresAt > currentTime) { + return cached.detail + } + if (cached) { + detailCache.delete(cacheKey) + } + + const pending = pendingDetailRequests.get(cacheKey) + if (pending) { + return pending + } + + const request = loader() + .then((detail) => { + detailCache.set(cacheKey, { + expiresAt: now() + (detail ? DETAIL_CACHE_TTL_MS : FAILURE_CACHE_TTL_MS), + detail, + }) + return detail + }) + .finally(() => { + pendingDetailRequests.delete(cacheKey) + }) + + pendingDetailRequests.set(cacheKey, request) + return request + } + + async function fetchBaseDetail(source: SkillMarketSource, slug: string): Promise { + if (source === 'clawhub') { try { - return await fetchClawHubDetail(slug, installed.has(slug)) + return await fetchClawHubDetail(slug) } catch (error) { if (!(error instanceof SkillMarketRequestError)) { throw error } } + + const list = await listClawHub({ + source: 'clawhub', + query: slug, + limit: MAX_LIMIT, + }) + const item = list.items.find((candidate) => candidate.source === source && candidate.slug === slug) + return item ? detailFromListItem(item) : null } - const list = await listSkills({ - source: params.source, + const list = await listSkillHub({ + source: 'skillhub', query: slug, limit: MAX_LIMIT, }) - const item = list.items.find((candidate) => candidate.source === params.source && candidate.slug === slug) - if (!item) { - return null - } - return detailFromListItem(item) + const item = list.items.find((candidate) => candidate.source === source && candidate.slug === slug) + return item ? detailFromListItem(item) : null } - async function fetchClawHubDetail(slug: string, installed: boolean): Promise { - const detailPayload = await requestJson(fetchImpl, clawHubDetailUrl(slug), 'ClawHub detail') - const scanPayload = await requestJson(fetchImpl, clawHubScanUrl(slug), 'ClawHub scan') - const detail = normalizeClawHubDetail(detailPayload, scanPayload, { installed }) + async function fetchClawHubDetail(slug: string): Promise { + const [detailPayload, scanPayload] = await Promise.all([ + requestJson(fetchImpl, clawHubDetailUrl(slug), 'ClawHub detail'), + requestJson(fetchImpl, clawHubScanUrl(slug), 'ClawHub scan'), + ]) + const detail = normalizeClawHubDetail(detailPayload, scanPayload, { installed: false }) if (!detail) { return null } @@ -302,24 +356,18 @@ export function createSkillMarketService(options: SkillMarketServiceOptions = {} version: string | undefined, files: SkillMarketFile[], ): Promise { - const previews: SkillMarketFilePreview[] = [] - for (const file of previewCandidateFiles(files)) { + const previews = await Promise.all(previewCandidateFiles(files).map(async (file) => { try { const content = await requestText(fetchImpl, clawHubFileUrl(slug, file.path, version), 'ClawHub file') - const preview = buildTextPreview(file, content) - if (preview) { - previews.push(preview) - } + return buildTextPreview(file, content) } catch (error) { if (!(error instanceof SkillMarketRequestError)) { throw error } + return null } - if (previews.length >= FILE_PREVIEW_LIMIT) { - break - } - } - return previews + })) + return previews.filter((preview): preview is SkillMarketFilePreview => preview !== null) } return { @@ -341,6 +389,32 @@ function detailFromListItem(item: SkillMarketItem): SkillMarketDetail { } } +function applyInstalledState(detail: SkillMarketDetail, installed: boolean): SkillMarketDetail { + if (installed) { + return { + ...detail, + installed: true, + installEligibility: { + status: 'installed', + installedSkillName: detail.slug, + }, + } + } + + if (!detail.installed) { + return detail + } + + return { + ...detail, + installed: false, + installEligibility: installEligibilityFromListItem({ + ...detail, + installed: false, + }), + } +} + function installEligibilityFromListItem(item: SkillMarketItem): SkillMarketDetail['installEligibility'] { if (item.installed) { return { @@ -414,6 +488,10 @@ function catalogCacheKey(source: 'clawhub' | 'skillhub', url: URL): string { return `${source}:${url.toString()}` } +function detailCacheKey(source: SkillMarketSource, slug: string): string { + return `${source}:${slug}` +} + function filterCatalogByQuery(result: SkillMarketListResult, query?: string): SkillMarketListResult { const terms = normalizeSearchTerms(query) if (terms.length === 0) {