diff --git a/src/renderer/src/components/skills/SkillCard.tsx b/src/renderer/src/components/skills/SkillCard.tsx new file mode 100644 index 000000000..6563f62fa --- /dev/null +++ b/src/renderer/src/components/skills/SkillCard.tsx @@ -0,0 +1,113 @@ +import { BookOpen, Clock, FolderOpen } from 'lucide-react' +import { toast } from 'sonner' +import { Badge } from '@/components/ui/badge' +import { Button } from '@/components/ui/button' +import { Card, CardContent } from '@/components/ui/card' +import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' +import { translate } from '@/i18n/i18n' +import type { DiscoveredSkill, SkillProvider } from '../../../../shared/skills' +import { pluralize, sourceLabels } from './skill-display-labels' + +const providerLabels: Record = { + codex: 'Codex', + claude: 'Claude', + 'agent-skills': 'Agent Skills' +} + +const dateFormatter = new Intl.DateTimeFormat(undefined, { + month: 'short', + day: 'numeric', + hour: 'numeric', + minute: '2-digit' +}) + +function formatUpdatedAt(value: number | null): string { + return value ? dateFormatter.format(new Date(value)) : 'Unknown' +} + +export function SkillCard({ skill }: { skill: DiscoveredSkill }): React.JSX.Element { + const revealSkill = async (): Promise => { + const result = await window.api.shell.openInFileManager(skill.skillFilePath) + if (!result.ok) { + toast.error( + translate('auto.components.skills.SkillsPage.995fde8337', 'Could not reveal skill file') + ) + } + } + + return ( + + +
+
+ +
+
+
+

{skill.name}

+ + {skill.installed + ? translate('auto.components.skills.SkillsPage.0c74e7ff34', 'Installed') + : translate('auto.components.skills.SkillsPage.35b9a724a0', 'Available')} + + + {sourceLabels[skill.sourceKind]} + +
+ {skill.description ? ( +

+ {skill.description} +

+ ) : ( +

+ {translate('auto.components.skills.SkillsPage.9963dff6d3', 'No description found.')} +

+ )} +
+ + + + + + {translate('auto.components.skills.SkillsPage.dc4c3328ee', 'Reveal file')} + + +
+ +
+
+ {skill.skillFilePath} +
+
+ {skill.providers.map((provider) => ( + + {providerLabels[provider]} + + ))} +
+
+ {skill.sourceLabel} + {pluralize(skill.fileCount, 'file')} + + + {formatUpdatedAt(skill.updatedAt)} + +
+
+
+
+ ) +} diff --git a/src/renderer/src/components/skills/SkillsPage.test.tsx b/src/renderer/src/components/skills/SkillsPage.test.tsx new file mode 100644 index 000000000..84bba21ef --- /dev/null +++ b/src/renderer/src/components/skills/SkillsPage.test.tsx @@ -0,0 +1,178 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { GlobalSettings } from '../../../../shared/types' +import type { DiscoveredSkill, SkillDiscoveryResult } from '../../../../shared/skills' +import { createCompatibleRuntimeStatusResponseIfNeeded } from '@/runtime/runtime-compatibility-test-fixture' +import { clearRuntimeCompatibilityCacheForTests } from '@/runtime/runtime-rpc-client' +import { TooltipProvider } from '@/components/ui/tooltip' +import { useAppStore } from '@/store' +import SkillsPage from './SkillsPage' + +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +let root: Root | null = null +let container: HTMLDivElement | null = null + +function skill(name: string): DiscoveredSkill { + return { + id: `skill-${name}`, + name, + description: null, + providers: ['agent-skills'], + sourceKind: 'home', + sourceLabel: 'Agent skills home', + rootPath: `/home/dev/.agents/skills`, + directoryPath: `/home/dev/.agents/skills/${name}`, + skillFilePath: `/home/dev/.agents/skills/${name}/SKILL.md`, + installed: true, + fileCount: 1, + updatedAt: null + } +} + +function discoveryResult(names: string[]): SkillDiscoveryResult { + return { skills: names.map(skill), sources: [], scannedAt: 1 } +} + +function deferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve!: (value: T) => void + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise + }) + return { promise, resolve } +} + +function setRuntimeOwner(environmentId: string | null): void { + useAppStore.setState({ + settings: { activeRuntimeEnvironmentId: environmentId } as GlobalSettings, + runtimeEnvironments: (environmentId ? [{ id: environmentId }] : []) as never, + runtimeEnvironmentCatalogSettled: true + }) +} + +async function renderPage(): Promise { + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + await act(async () => { + root?.render( + + + + ) + }) +} + +async function flushMicrotasks(): Promise { + await act(async () => { + for (let tick = 0; tick < 8; tick += 1) { + await Promise.resolve() + } + }) +} + +/** Skill names currently rendered as cards. */ +function renderedSkillNames(): string[] { + return [...(container?.querySelectorAll('h3') ?? [])].map((node) => node.textContent ?? '') +} + +beforeEach(() => { + setRuntimeOwner(null) +}) + +afterEach(async () => { + if (root) { + await act(async () => { + root?.unmount() + }) + } + root = null + container?.remove() + container = null + clearRuntimeCompatibilityCacheForTests() + useAppStore.setState({ + settings: null, + runtimeEnvironments: [], + runtimeEnvironmentCatalogSettled: false + }) + vi.restoreAllMocks() + Reflect.deleteProperty(window, 'api') +}) + +describe('SkillsPage', () => { + it('scans the connected remote runtime instead of the client disk', async () => { + const discover = vi.fn().mockResolvedValue(discoveryResult(['local-only'])) + const call = vi.fn( + async (args: { method: string; selector?: string }) => + createCompatibleRuntimeStatusResponseIfNeeded(args) ?? { + id: 'skills', + ok: true, + result: discoveryResult(['remote-only']) + } + ) + Object.defineProperty(window, 'api', { + configurable: true, + value: { skills: { discover }, runtimeEnvironments: { call } } + }) + setRuntimeOwner('env-1') + + await renderPage() + await flushMicrotasks() + + expect(discover).not.toHaveBeenCalled() + expect(renderedSkillNames()).toContain('remote-only') + }) + + // Why: a cold local scan walks every skill root, so it can land after a newer + // remote scan. Without a generation guard it overwrites the remote list and + // the page silently shows the client's skills again — #6789 all over. + it('does not let a slow local scan overwrite a newer remote scan', async () => { + const localScan = deferred() + const discover = vi.fn().mockReturnValue(localScan.promise) + const call = vi.fn( + async (args: { method: string; selector?: string }) => + createCompatibleRuntimeStatusResponseIfNeeded(args) ?? { + id: 'skills', + ok: true, + result: discoveryResult(['remote-only']) + } + ) + Object.defineProperty(window, 'api', { + configurable: true, + value: { skills: { discover }, runtimeEnvironments: { call } } + }) + + await renderPage() + await act(async () => { + setRuntimeOwner('env-1') + }) + await flushMicrotasks() + expect(renderedSkillNames()).toContain('remote-only') + + localScan.resolve(discoveryResult(['local-only'])) + await flushMicrotasks() + + expect(renderedSkillNames()).toContain('remote-only') + expect(renderedSkillNames()).not.toContain('local-only') + }) + + it('keeps scanning rather than listing client skills before the owner is known', async () => { + const discover = vi.fn().mockResolvedValue(discoveryResult(['local-only'])) + const call = vi.fn() + Object.defineProperty(window, 'api', { + configurable: true, + value: { skills: { discover }, runtimeEnvironments: { call } } + }) + useAppStore.setState({ runtimeEnvironmentCatalogSettled: false }) + + await renderPage() + await flushMicrotasks() + + expect(discover).not.toHaveBeenCalled() + expect(call).not.toHaveBeenCalled() + expect(container?.textContent).toContain('Scanning skills') + }) +}) diff --git a/src/renderer/src/components/skills/SkillsPage.tsx b/src/renderer/src/components/skills/SkillsPage.tsx index 302e16f48..70b779591 100644 --- a/src/renderer/src/components/skills/SkillsPage.tsx +++ b/src/renderer/src/components/skills/SkillsPage.tsx @@ -1,9 +1,8 @@ -import { useCallback, useEffect, useMemo, useState } from 'react' -import { ArrowLeft, BookOpen, Clock, FolderOpen, Loader2, RefreshCw, Search } from 'lucide-react' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { ArrowLeft, BookOpen, Loader2, RefreshCw, Search } from 'lucide-react' import { toast } from 'sonner' import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' -import { Card, CardContent } from '@/components/ui/card' import { Input } from '@/components/ui/input' import { Select, @@ -12,136 +11,19 @@ import { SelectTrigger, SelectValue } from '@/components/ui/select' -import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { cn } from '@/lib/utils' import { useAppStore } from '@/store' +import { discoverSkillsForRuntimeTarget } from '@/runtime/runtime-skills-client' +import { useActiveSkillDiscoveryRuntimeTarget } from '@/hooks/use-active-skill-discovery-runtime-target' import { useMountedRef } from '@/hooks/useMountedRef' -import type { - DiscoveredSkill, - SkillDiscoveryResult, - SkillProvider, - SkillSourceKind -} from '../../../../shared/skills' +import type { DiscoveredSkill, SkillDiscoveryResult } from '../../../../shared/skills' +import { SkillCard } from './SkillCard' +import { pluralize, sourceLabels } from './skill-display-labels' import { countSkillsBySource, filterSkills, type SkillsFilterState } from './skills-filter' import { translate } from '@/i18n/i18n' -const providerLabels: Record = { - codex: 'Codex', - claude: 'Claude', - 'agent-skills': 'Agent Skills' -} - -const sourceLabels: Record = { - home: 'Home', - repo: 'Repository', - bundled: 'Bundled', - plugin: 'Plugin' -} - -const dateFormatter = new Intl.DateTimeFormat(undefined, { - month: 'short', - day: 'numeric', - hour: 'numeric', - minute: '2-digit' -}) - const EMPTY_SKILLS: DiscoveredSkill[] = [] -function formatUpdatedAt(value: number | null): string { - return value ? dateFormatter.format(new Date(value)) : 'Unknown' -} - -function pluralize(count: number, singular: string): string { - return `${count} ${singular}${count === 1 ? '' : 's'}` -} - -function SkillCard({ skill }: { skill: DiscoveredSkill }): React.JSX.Element { - const revealSkill = async (): Promise => { - const result = await window.api.shell.openInFileManager(skill.skillFilePath) - if (!result.ok) { - toast.error( - translate('auto.components.skills.SkillsPage.995fde8337', 'Could not reveal skill file') - ) - } - } - - return ( - - -
-
- -
-
-
-

{skill.name}

- - {skill.installed - ? translate('auto.components.skills.SkillsPage.0c74e7ff34', 'Local') - : translate('auto.components.skills.SkillsPage.35b9a724a0', 'Available')} - - - {sourceLabels[skill.sourceKind]} - -
- {skill.description ? ( -

- {skill.description} -

- ) : ( -

- {translate('auto.components.skills.SkillsPage.9963dff6d3', 'No description found.')} -

- )} -
- - - - - - {translate('auto.components.skills.SkillsPage.dc4c3328ee', 'Reveal file')} - - -
- -
-
- {skill.skillFilePath} -
-
- {skill.providers.map((provider) => ( - - {providerLabels[provider]} - - ))} -
-
- {skill.sourceLabel} - {pluralize(skill.fileCount, 'file')} - - - {formatUpdatedAt(skill.updatedAt)} - -
-
-
-
- ) -} - function EmptyState({ loading, hasSkills, @@ -165,10 +47,7 @@ function EmptyState({ ? translate('auto.components.skills.SkillsPage.cd7893fbc1', 'Scanning skills') : hasSkills ? translate('auto.components.skills.SkillsPage.6a62a0168c', 'No matches') - : translate( - 'auto.components.skills.SkillsPage.4acd6d68ec', - 'No local skills found' - )} + : translate('auto.components.skills.SkillsPage.4acd6d68ec', 'No skills found')}

{hasSkills @@ -178,7 +57,7 @@ function EmptyState({ ) : translate( 'auto.components.skills.SkillsPage.ab5b777350', - 'Checked local home, repository, bundled, and plugin skill folders.' + 'Checked home, repository, bundled, and plugin skill folders.' )}

@@ -195,6 +74,7 @@ function EmptyState({ export default function SkillsPage(): React.JSX.Element { const closeSkillsPage = useAppStore((s) => s.closeSkillsPage) + const runtimeTarget = useActiveSkillDiscoveryRuntimeTarget() const [result, setResult] = useState(null) const [loading, setLoading] = useState(true) const [filters, setFilters] = useState({ @@ -203,27 +83,38 @@ export default function SkillsPage(): React.JSX.Element { provider: 'all' }) const mountedRef = useMountedRef() + const scanGenerationRef = useRef(0) const loadSkills = useCallback(async (): Promise => { setLoading(true) + // Why: a cold local scan walks every skill root, so switching runtimes can + // land a stale result after a newer one. Only the newest scan may write. + const scanGeneration = ++scanGenerationRef.current + const isCurrentScan = (): boolean => + mountedRef.current && scanGeneration === scanGenerationRef.current + if (!runtimeTarget) { + // Why: keep scanning until the owning runtime is known, rather than + // showing the client's skills to someone whose skills live remotely. + return + } try { - const nextResult = await window.api.skills.discover() - if (mountedRef.current) { + const nextResult = await discoverSkillsForRuntimeTarget(runtimeTarget) + if (isCurrentScan()) { setResult(nextResult) } } catch (error) { console.error('Failed to discover skills:', error) - if (mountedRef.current) { + if (isCurrentScan()) { toast.error( - translate('auto.components.skills.SkillsPage.ea72d6185b', 'Could not scan local skills') + translate('auto.components.skills.SkillsPage.ea72d6185b', 'Could not scan skills') ) } } finally { - if (mountedRef.current) { + if (isCurrentScan()) { setLoading(false) } } - }, [mountedRef]) + }, [mountedRef, runtimeTarget]) useEffect(() => { void loadSkills() diff --git a/src/renderer/src/components/skills/skill-display-labels.ts b/src/renderer/src/components/skills/skill-display-labels.ts new file mode 100644 index 000000000..9a2dd104b --- /dev/null +++ b/src/renderer/src/components/skills/skill-display-labels.ts @@ -0,0 +1,12 @@ +import type { SkillSourceKind } from '../../../../shared/skills' + +export const sourceLabels: Record = { + home: 'Home', + repo: 'Repository', + bundled: 'Bundled', + plugin: 'Plugin' +} + +export function pluralize(count: number, singular: string): string { + return `${count} ${singular}${count === 1 ? '' : 's'}` +} diff --git a/src/renderer/src/hooks/installed-agent-skill-discovery.ts b/src/renderer/src/hooks/installed-agent-skill-discovery.ts new file mode 100644 index 000000000..7ad435c46 --- /dev/null +++ b/src/renderer/src/hooks/installed-agent-skill-discovery.ts @@ -0,0 +1,138 @@ +import type { SkillDiscoveryResult, SkillDiscoveryTarget } from '../../../shared/skills' +import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' +import { discoverSkillsForRuntimeTarget } from '@/runtime/runtime-skills-client' +import { INSTALLED_AGENT_SKILLS_CHANGED_EVENT } from './installed-agent-skills-change-event' + +export const LOCAL_RUNTIME_TARGET: RuntimeClientTarget = { kind: 'local' } + +let cachedDiscoveryByTarget = new Map() +let pendingDiscoveryByTarget = new Map>() +let pendingDiscoverySatisfiesForcedRefreshByTarget = new Map() + +/** Last completed scan for a runtime-scoped key, for a synchronous first render. */ +export function getCachedSkillDiscovery(key: string): SkillDiscoveryResult | null { + return cachedDiscoveryByTarget.get(key) ?? null +} + +/** Invalidate every cached scan and tell mounted hooks to re-scan (e.g. after an install). */ +export function notifyInstalledAgentSkillsChanged(): void { + cachedDiscoveryByTarget.clear() + if (typeof window !== 'undefined') { + window.dispatchEvent(new CustomEvent(INSTALLED_AGENT_SKILLS_CHANGED_EVENT)) + } +} + +export function resetSkillDiscoveryCacheForTests(): void { + cachedDiscoveryByTarget = new Map() + pendingDiscoveryByTarget = new Map() + pendingDiscoverySatisfiesForcedRefreshByTarget = new Map() +} + +function normalizeSkillDiscoveryTarget( + target: SkillDiscoveryTarget | undefined +): SkillDiscoveryTarget | undefined { + const projectRuntime = target?.projectRuntime + if (projectRuntime) { + if (projectRuntime.status === 'repair-required') { + return { projectRuntime } + } + if (projectRuntime.runtime.kind === 'wsl') { + return { + runtime: 'wsl', + wslDistro: projectRuntime.runtime.distro, + projectRuntime + } + } + return { + runtime: 'host', + projectRuntime + } + } + + if (target?.runtime !== 'wsl') { + return undefined + } + return { runtime: 'wsl', wslDistro: target.wslDistro?.trim() || null } +} + +export function getSkillDiscoveryTargetKey(target: SkillDiscoveryTarget | undefined): string { + if (target?.projectRuntime) { + return target.projectRuntime.status === 'resolved' + ? target.projectRuntime.runtime.cacheKey + : target.projectRuntime.repair.cacheKey + } + const normalizedTarget = normalizeSkillDiscoveryTarget(target) + return normalizedTarget?.runtime === 'wsl' ? `wsl:${normalizedTarget.wslDistro ?? ''}` : 'host' +} + +// Why: a connected remote runtime scans its own disk. Sharing the local key +// would keep showing the client's skills after switching environments. The +// caller's target is dropped for a remote scan (it describes the client's WSL / +// project runtime), so it must not fragment the key either — otherwise the same +// remote gets rescanned once per client-side target shape. +export function getRuntimeScopedSkillDiscoveryKey( + runtimeTarget: RuntimeClientTarget, + target: SkillDiscoveryTarget | undefined +): string { + return runtimeTarget.kind === 'environment' + ? `runtime:${runtimeTarget.environmentId}` + : getSkillDiscoveryTargetKey(target) +} + +function startInstalledAgentSkillDiscovery( + force: boolean, + target: SkillDiscoveryTarget | undefined, + runtimeTarget: RuntimeClientTarget +): Promise { + const key = getRuntimeScopedSkillDiscoveryKey(runtimeTarget, target) + const normalizedTarget = normalizeSkillDiscoveryTarget(target) + const discovery = discoverSkillsForRuntimeTarget(runtimeTarget, normalizedTarget) + .then((result) => { + cachedDiscoveryByTarget.set(key, result) + return result + }) + .finally(() => { + if (pendingDiscoveryByTarget.get(key) === discovery) { + pendingDiscoveryByTarget.delete(key) + pendingDiscoverySatisfiesForcedRefreshByTarget.delete(key) + } + }) + pendingDiscoveryByTarget.set(key, discovery) + pendingDiscoverySatisfiesForcedRefreshByTarget.set(key, force) + return discovery +} + +/** + * Cached, de-duplicated skill scan for one runtime. Concurrent callers share a + * single in-flight scan per key; `force` bypasses the cache to re-read disk. + */ +export async function discoverInstalledAgentSkills( + force: boolean, + target?: SkillDiscoveryTarget, + runtimeTarget: RuntimeClientTarget = LOCAL_RUNTIME_TARGET +): Promise { + const key = getRuntimeScopedSkillDiscoveryKey(runtimeTarget, target) + const cachedDiscovery = cachedDiscoveryByTarget.get(key) + if (!force && cachedDiscovery) { + return cachedDiscovery + } + + const inFlightDiscovery = pendingDiscoveryByTarget.get(key) + if (inFlightDiscovery) { + if (!force || pendingDiscoverySatisfiesForcedRefreshByTarget.get(key)) { + return inFlightDiscovery + } + try { + await inFlightDiscovery + } catch { + // Why: an explicit re-check should still read current disk state even if + // the older background scan failed. + } + const nextPendingDiscovery = pendingDiscoveryByTarget.get(key) + if (nextPendingDiscovery && nextPendingDiscovery !== inFlightDiscovery) { + return nextPendingDiscovery + } + } + + return startInstalledAgentSkillDiscovery(force, target, runtimeTarget) +} diff --git a/src/renderer/src/hooks/use-active-skill-discovery-runtime-target.ts b/src/renderer/src/hooks/use-active-skill-discovery-runtime-target.ts new file mode 100644 index 000000000..214526bdd --- /dev/null +++ b/src/renderer/src/hooks/use-active-skill-discovery-runtime-target.ts @@ -0,0 +1,40 @@ +import { useMemo } from 'react' +import { getSingleFocusedRuntimeEnvironmentId } from '@/lib/single-runtime-legacy-owner' +import { getActiveRuntimeTarget, type RuntimeClientTarget } from '@/runtime/runtime-rpc-client' +import { useAppStore } from '@/store' + +/** Distinguishes "not known yet" from "known to be the local host", without + * colliding with an environment id that happens to be named the same. */ +const UNRESOLVED = Symbol('skill-discovery-runtime-unresolved') + +/** + * Runtime that owns skill discovery, or `null` while the store still cannot say. + * + * Resolved through `getSingleFocusedRuntimeEnvironmentId` rather than raw + * `activeRuntimeEnvironmentId` on purpose: the skill *install* terminal routes + * through that same resolver (`terminal-worktree-route.ts`), which declines to + * guess an owner while several runtimes are saved. Scanning a host the install + * cannot reach would leave the badge stuck on "Not installed" forever — #6789 + * again, just inverted. Scan and install must always name the same host. + */ +export function useActiveSkillDiscoveryRuntimeTarget(): RuntimeClientTarget | null { + // Why: select the resolved id (a string) rather than its inputs. Selecting + // `runtimeEnvironments` would churn identity every time a status refresh + // restores an equal-but-new array, re-firing every consumer's scan. + const environmentId = useAppStore((state) => + // Why: resolving to "local" before the catalog settles caches a client scan + // under the local key and flashes "Not installed" at a user whose skills live + // remotely. Settled rather than hydrated, so a failed catalog read degrades to + // the local host instead of leaving every badge pending for the session. + state.runtimeEnvironmentCatalogSettled + ? getSingleFocusedRuntimeEnvironmentId(state) + : UNRESOLVED + ) + return useMemo( + () => + environmentId === UNRESOLVED + ? null + : getActiveRuntimeTarget({ activeRuntimeEnvironmentId: environmentId }), + [environmentId] + ) +} diff --git a/src/renderer/src/hooks/useInstalledAgentSkills.react.test.tsx b/src/renderer/src/hooks/useInstalledAgentSkills.react.test.tsx index 48274dd27..218155ea8 100644 --- a/src/renderer/src/hooks/useInstalledAgentSkills.react.test.tsx +++ b/src/renderer/src/hooks/useInstalledAgentSkills.react.test.tsx @@ -2,13 +2,17 @@ import { act } from 'react' import { createRoot, type Root } from 'react-dom/client' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { DiscoveredSkill, SkillDiscoveryResult, SkillDiscoveryTarget } from '../../../shared/skills' import type { ProjectExecutionRuntimeResolution } from '../../../shared/project-execution-runtime' +import type { GlobalSettings } from '../../../shared/types' +import { createCompatibleRuntimeStatusResponseIfNeeded } from '@/runtime/runtime-compatibility-test-fixture' +import { clearRuntimeCompatibilityCacheForTests } from '@/runtime/runtime-rpc-client' +import { useAppStore } from '@/store' import { GLOBAL_AGENT_SKILL_SOURCE_KINDS, type InstalledAgentSkillState, @@ -104,10 +108,44 @@ afterEach(async () => { container = null latestState = null _installedAgentSkillDiscoveryInternalsForTests.reset() + clearRuntimeCompatibilityCacheForTests() + useAppStore.setState({ + settings: null, + runtimeEnvironments: [], + runtimeEnvironmentCatalogSettled: false + }) vi.restoreAllMocks() Reflect.deleteProperty(window, 'api') }) +/** Drain the compat probe + RPC promise chain a remote scan walks before it lands in state. */ +async function flushMicrotasks(): Promise { + await act(async () => { + for (let tick = 0; tick < 8; tick += 1) { + await Promise.resolve() + } + }) +} + +/** + * Hydrate the store the way a running app does. `savedEnvironmentIds` defaults to + * just the focused one, which is the only shape that resolves to a remote owner. + */ +function setRuntimeOwner( + environmentId: string | null, + savedEnvironmentIds: readonly string[] = environmentId ? [environmentId] : [] +): void { + useAppStore.setState({ + settings: { activeRuntimeEnvironmentId: environmentId } as GlobalSettings, + runtimeEnvironments: savedEnvironmentIds.map((id) => ({ id })) as never, + runtimeEnvironmentCatalogSettled: true + }) +} + +beforeEach(() => { + setRuntimeOwner(null) +}) + describe('useInstalledAgentSkill', () => { it('ignores stale discovery results after the discovery target changes', async () => { const hostScan = deferred() @@ -245,4 +283,115 @@ describe('useInstalledAgentSkill', () => { projectRuntime: projectWslRuntime }) }) + + it('scans the connected remote runtime and keeps that result out of the local cache', async () => { + const discover = vi + .fn<(target?: SkillDiscoveryTarget) => Promise>() + .mockResolvedValue(discoveryResult([])) + const call = vi.fn( + async (args: { method: string; selector?: string }) => + createCompatibleRuntimeStatusResponseIfNeeded(args) ?? { + id: 'skills', + ok: true, + result: discoveryResult([skill({ name: 'linear-tickets' })]) + } + ) + Object.defineProperty(window, 'api', { + configurable: true, + value: { skills: { discover }, runtimeEnvironments: { call } } + }) + setRuntimeOwner('env-1') + + await renderProbe() + await flushMicrotasks() + + expect(latestState?.installed).toBe(true) + expect(discover).not.toHaveBeenCalled() + expect(call).toHaveBeenCalledWith( + expect.objectContaining({ selector: 'env-1', method: 'skills.discover' }) + ) + + // Why: the remote hit is keyed per environment, so switching back to the + // local host must re-scan the client instead of replaying the server's list. + await act(async () => { + setRuntimeOwner(null) + }) + await flushMicrotasks() + + expect(discover).toHaveBeenCalledTimes(1) + expect(latestState?.installed).toBe(false) + }) + + // Why: the skill INSTALL terminal routes through getSingleFocusedRuntimeEnvironmentId, + // which refuses to guess an owner while several runtimes are saved. Scanning the + // focused remote here would leave the badge stuck on "Not installed" forever, + // because the install actually lands on the local client. + it('scans the local host when several saved runtimes make the install host ambiguous', async () => { + const discover = vi + .fn<(target?: SkillDiscoveryTarget) => Promise>() + .mockResolvedValue(discoveryResult([skill({ name: 'linear-tickets' })])) + const call = vi.fn() + Object.defineProperty(window, 'api', { + configurable: true, + value: { skills: { discover }, runtimeEnvironments: { call } } + }) + setRuntimeOwner('env-1', ['env-1', 'env-2']) + + await renderProbe() + await flushMicrotasks() + + expect(call).not.toHaveBeenCalled() + expect(discover).toHaveBeenCalledTimes(1) + expect(latestState?.installed).toBe(true) + }) + + it('keeps loading instead of scanning the wrong host before the catalog settles', async () => { + const discover = vi + .fn<(target?: SkillDiscoveryTarget) => Promise>() + .mockResolvedValue(discoveryResult([])) + const call = vi.fn() + Object.defineProperty(window, 'api', { + configurable: true, + value: { skills: { discover }, runtimeEnvironments: { call } } + }) + // Why: a focused remote is already known here, so a missing gate resolves to + // the local host and caches a client scan under the local key. + useAppStore.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' } as GlobalSettings, + runtimeEnvironments: [{ id: 'env-1' }] as never, + runtimeEnvironmentCatalogSettled: false + }) + + await renderProbe() + await flushMicrotasks() + + expect(discover).not.toHaveBeenCalled() + expect(call).not.toHaveBeenCalled() + expect(latestState?.loading).toBe(true) + expect(latestState?.installed).toBe(false) + }) + + // Why: a failed catalog read must degrade to the local host, not strand every + // skill badge on a spinner with no retry affordance for the whole session. + it('falls back to the local host once an unreadable catalog settles', async () => { + const discover = vi + .fn<(target?: SkillDiscoveryTarget) => Promise>() + .mockResolvedValue(discoveryResult([skill({ name: 'linear-tickets' })])) + Object.defineProperty(window, 'api', { + configurable: true, + value: { skills: { discover }, runtimeEnvironments: { call: vi.fn() } } + }) + useAppStore.setState({ + settings: null, + runtimeEnvironments: [], + runtimeEnvironmentCatalogSettled: true + }) + + await renderProbe() + await flushMicrotasks() + + expect(discover).toHaveBeenCalledTimes(1) + expect(latestState?.loading).toBe(false) + expect(latestState?.installed).toBe(true) + }) }) diff --git a/src/renderer/src/hooks/useInstalledAgentSkills.ts b/src/renderer/src/hooks/useInstalledAgentSkills.ts index 6d2afc40e..0ed5e4461 100644 --- a/src/renderer/src/hooks/useInstalledAgentSkills.ts +++ b/src/renderer/src/hooks/useInstalledAgentSkills.ts @@ -8,9 +8,22 @@ import type { } from '../../../shared/skills' import { ORCHESTRATION_SKILL_NAME } from '@/lib/agent-feature-install-commands' import { markOrchestrationSetupComplete } from '@/lib/orchestration-setup-state' +import { + discoverInstalledAgentSkills, + getCachedSkillDiscovery, + getRuntimeScopedSkillDiscoveryKey, + getSkillDiscoveryTargetKey, + resetSkillDiscoveryCacheForTests +} from './installed-agent-skill-discovery' import { INSTALLED_AGENT_SKILLS_CHANGED_EVENT } from './installed-agent-skills-change-event' +import { useActiveSkillDiscoveryRuntimeTarget } from './use-active-skill-discovery-runtime-target' import { useMountedRef } from './useMountedRef' +/** Placeholder key while the owning runtime is unknown; nothing is cached under it. */ +const UNRESOLVED_RUNTIME_DISCOVERY_KEY = 'runtime:unresolved' + +export { notifyInstalledAgentSkillsChanged } from './installed-agent-skill-discovery' + export const GLOBAL_AGENT_SKILL_SOURCE_KINDS = [ 'home' ] as const satisfies readonly SkillSourceKind[] @@ -34,10 +47,6 @@ export type InstalledAgentSkillState = { refresh: () => Promise } -let cachedDiscoveryByTarget = new Map() -let pendingDiscoveryByTarget = new Map>() -let pendingDiscoverySatisfiesForcedRefreshByTarget = new Map() - function normalizeSkillName(value: string): string { return value.trim().toLowerCase() } @@ -78,112 +87,11 @@ export function hasInstalledAgentSkillNamed( }) } -export function notifyInstalledAgentSkillsChanged(): void { - cachedDiscoveryByTarget.clear() - if (typeof window !== 'undefined') { - window.dispatchEvent(new CustomEvent(INSTALLED_AGENT_SKILLS_CHANGED_EVENT)) - } -} - -function normalizeSkillDiscoveryTarget( - target: SkillDiscoveryTarget | undefined -): SkillDiscoveryTarget | undefined { - const projectRuntime = target?.projectRuntime - if (projectRuntime) { - if (projectRuntime.status === 'repair-required') { - return { projectRuntime } - } - if (projectRuntime.runtime.kind === 'wsl') { - return { - runtime: 'wsl', - wslDistro: projectRuntime.runtime.distro, - projectRuntime - } - } - return { - runtime: 'host', - projectRuntime - } - } - - if (target?.runtime !== 'wsl') { - return undefined - } - return { runtime: 'wsl', wslDistro: target.wslDistro?.trim() || null } -} - -function getSkillDiscoveryTargetKey(target: SkillDiscoveryTarget | undefined): string { - if (target?.projectRuntime) { - return target.projectRuntime.status === 'resolved' - ? target.projectRuntime.runtime.cacheKey - : target.projectRuntime.repair.cacheKey - } - const normalizedTarget = normalizeSkillDiscoveryTarget(target) - return normalizedTarget?.runtime === 'wsl' ? `wsl:${normalizedTarget.wslDistro ?? ''}` : 'host' -} - -function startInstalledAgentSkillDiscovery( - force: boolean, - target: SkillDiscoveryTarget | undefined -): Promise { - const key = getSkillDiscoveryTargetKey(target) - const normalizedTarget = normalizeSkillDiscoveryTarget(target) - const discovery = window.api.skills - .discover(normalizedTarget) - .then((result) => { - cachedDiscoveryByTarget.set(key, result) - return result - }) - .finally(() => { - if (pendingDiscoveryByTarget.get(key) === discovery) { - pendingDiscoveryByTarget.delete(key) - pendingDiscoverySatisfiesForcedRefreshByTarget.delete(key) - } - }) - pendingDiscoveryByTarget.set(key, discovery) - pendingDiscoverySatisfiesForcedRefreshByTarget.set(key, force) - return discovery -} - -async function discoverInstalledAgentSkills( - force: boolean, - target?: SkillDiscoveryTarget -): Promise { - const key = getSkillDiscoveryTargetKey(target) - const cachedDiscovery = cachedDiscoveryByTarget.get(key) - if (!force && cachedDiscovery) { - return cachedDiscovery - } - - const inFlightDiscovery = pendingDiscoveryByTarget.get(key) - if (inFlightDiscovery) { - if (!force || pendingDiscoverySatisfiesForcedRefreshByTarget.get(key)) { - return inFlightDiscovery - } - try { - await inFlightDiscovery - } catch { - // Why: an explicit re-check should still read current disk state even if - // the older background scan failed. - } - const nextPendingDiscovery = pendingDiscoveryByTarget.get(key) - if (nextPendingDiscovery && nextPendingDiscovery !== inFlightDiscovery) { - return nextPendingDiscovery - } - } - - return startInstalledAgentSkillDiscovery(force, target) -} - export const _installedAgentSkillDiscoveryInternalsForTests = { discoverInstalledAgentSkills, getSkillDiscoveryTargetKey, isOrchestrationSkillName, - reset(): void { - cachedDiscoveryByTarget = new Map() - pendingDiscoveryByTarget = new Map() - pendingDiscoverySatisfiesForcedRefreshByTarget = new Map() - } + reset: resetSkillDiscoveryCacheForTests } export function useInstalledAgentSkill( @@ -200,8 +108,11 @@ export function useInstalledAgentSkillNames( const { enabled = true, discoveryTarget, sourceKinds } = options const skillNamesKey = skillNames.map(normalizeSkillName).join('\n') const candidateSkillNames = useMemo(() => skillNamesKey.split('\n'), [skillNamesKey]) - const discoveryTargetKey = getSkillDiscoveryTargetKey(discoveryTarget) - const cachedDiscovery = cachedDiscoveryByTarget.get(discoveryTargetKey) ?? null + const runtimeTarget = useActiveSkillDiscoveryRuntimeTarget() + const discoveryTargetKey = runtimeTarget + ? getRuntimeScopedSkillDiscoveryKey(runtimeTarget, discoveryTarget) + : UNRESOLVED_RUNTIME_DISCOVERY_KEY + const cachedDiscovery = getCachedSkillDiscovery(discoveryTargetKey) const [result, setResult] = useState(cachedDiscovery) const [loading, setLoading] = useState(enabled && !cachedDiscovery) const [error, setError] = useState(null) @@ -219,7 +130,7 @@ export function useInstalledAgentSkillNames( stateResetInputRef.current.discoveryTargetKey !== discoveryTargetKey || stateResetInputRef.current.enabled !== enabled ) { - const nextCachedDiscovery = cachedDiscoveryByTarget.get(discoveryTargetKey) ?? null + const nextCachedDiscovery = getCachedSkillDiscovery(discoveryTargetKey) const nextLoading = enabled && !nextCachedDiscovery stateResetInputRef.current = { discoveryTargetKey, enabled } resultForRender = nextCachedDiscovery @@ -253,9 +164,14 @@ export function useInstalledAgentSkillNames( writeIfCurrent(() => { setLoading(true) }) + if (!runtimeTarget) { + // Why: stay in the loading state rather than scanning the wrong host and + // reporting "not installed" before the owning runtime is known. + return false + } let installedAfterRefresh = false try { - const next = await discoverInstalledAgentSkills(force, discoveryTarget) + const next = await discoverInstalledAgentSkills(force, discoveryTarget, runtimeTarget) installedAfterRefresh = hasInstalledAgentSkillNamed(next.skills, candidateSkillNames, { sourceKinds }) @@ -278,7 +194,15 @@ export function useInstalledAgentSkillNames( } return installedAfterRefresh }, - [candidateSkillNames, discoveryTarget, discoveryTargetKey, enabled, mountedRef, sourceKinds] + [ + candidateSkillNames, + discoveryTarget, + discoveryTargetKey, + enabled, + mountedRef, + runtimeTarget, + sourceKinds + ] ) useEffect(() => { diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index a5f34ab6c..3de51346c 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -3786,17 +3786,17 @@ "b088e0785d": "Beta", "f43ad6edf3": "Skills", "7e828fb2c6": "Back", - "ea72d6185b": "Could not scan local skills", + "ea72d6185b": "Could not scan skills", "dc4c3328ee": "Reveal file", "9963dff6d3": "No description found.", "995fde8337": "Could not reveal skill file", - "ab5b777350": "Checked local home, repository, bundled, and plugin skill folders.", + "ab5b777350": "Checked home, repository, bundled, and plugin skill folders.", "08a321a984": "Adjust the search or filters.", - "4acd6d68ec": "No local skills found", + "4acd6d68ec": "No skills found", "6a62a0168c": "No matches", "cd7893fbc1": "Scanning skills", "35b9a724a0": "Available", - "0c74e7ff34": "Local" + "0c74e7ff34": "Installed" }, "SkillFreshnessNudge": { "titleOne": "An installed Orca skill is out of date", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index a3052daca..2edcded71 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -3763,17 +3763,17 @@ "b088e0785d": "Beta", "f43ad6edf3": "Skills", "7e828fb2c6": "Volver", - "ea72d6185b": "No se pudieron escanear los skills locales", + "ea72d6185b": "No se pudieron escanear los skills", "dc4c3328ee": "Mostrar archivo", "9963dff6d3": "No se encontró ninguna descripción.", "995fde8337": "No se pudo mostrar el archivo del skill", - "ab5b777350": "Se comprobaron las carpetas de skills locales, del repositorio, incluidas y de plugins.", + "ab5b777350": "Se comprobaron las carpetas de skills del home, del repositorio, incluidas y de plugins.", "08a321a984": "Ajusta la búsqueda o los filtros.", - "4acd6d68ec": "No se encontraron skills locales", + "4acd6d68ec": "No se encontraron skills", "6a62a0168c": "No hay coincidencias", "cd7893fbc1": "Escaneando skills", "35b9a724a0": "Disponible", - "0c74e7ff34": "Local" + "0c74e7ff34": "Instalado" }, "SkillFreshnessNudge": { "titleOne": "Una skill de Orca instalada está desactualizada", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 72a67e576..fffa62c2f 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -3763,17 +3763,17 @@ "b088e0785d": "ベータ", "f43ad6edf3": "スキル", "7e828fb2c6": "戻る", - "ea72d6185b": "ローカルスキルをスキャンできませんでした", + "ea72d6185b": "スキルをスキャンできませんでした", "dc4c3328ee": "ファイルを公開する", "9963dff6d3": "説明が見つかりませんでした。", "995fde8337": "スキルファイルを公開できませんでした", - "ab5b777350": "ローカル ホーム、repos、バンドル、プラグイン スキル フォルダを確認しました。", + "ab5b777350": "ホーム、repos、バンドル、プラグイン スキル フォルダを確認しました。", "08a321a984": "検索またはフィルターを調整します。", - "4acd6d68ec": "ローカルスキルが見つかりません", + "4acd6d68ec": "スキルが見つかりません", "6a62a0168c": "一致なし", "cd7893fbc1": "スキャンスキル", "35b9a724a0": "利用可能", - "0c74e7ff34": "ローカル" + "0c74e7ff34": "インストール済み" }, "SkillFreshnessNudge": { "titleOne": "インストール済みの Orca スキルが古くなっています", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 900174621..d96497fcd 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -3763,17 +3763,17 @@ "b088e0785d": "베타", "f43ad6edf3": "스킬", "7e828fb2c6": "뒤로", - "ea72d6185b": "로컬 스킬을 스캔할 수 없습니다.", + "ea72d6185b": "스킬을 스캔할 수 없습니다.", "dc4c3328ee": "파일 표시", "9963dff6d3": "설명을 찾을 수 없습니다.", "995fde8337": "스킬 파일을 공개할 수 없습니다.", - "ab5b777350": "로컬 홈, repos, 번들, 플러그인 스킬 폴더를 확인했습니다.", + "ab5b777350": "홈, repos, 번들, 플러그인 스킬 폴더를 확인했습니다.", "08a321a984": "검색 또는 필터를 조정합니다.", - "4acd6d68ec": "로컬 스킬을 찾을 수 없습니다.", + "4acd6d68ec": "스킬을 찾을 수 없습니다.", "6a62a0168c": "일치하는 항목 없음", "cd7893fbc1": "스킬 스캔 중", "35b9a724a0": "사용 가능", - "0c74e7ff34": "로컬" + "0c74e7ff34": "설치됨" }, "SkillFreshnessNudge": { "titleOne": "설치된 Orca 스킬이 오래되었습니다", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index b4d759477..d24db028e 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -3763,17 +3763,17 @@ "b088e0785d": "测试版", "f43ad6edf3": "技能", "7e828fb2c6": "返回", - "ea72d6185b": "无法扫描本地技能", + "ea72d6185b": "无法扫描技能", "dc4c3328ee": "显示文件", "9963dff6d3": "没有找到描述。", "995fde8337": "无法显示技能文件", - "ab5b777350": "检查本地主目录、存储库、捆绑和插件技能文件夹。", + "ab5b777350": "检查主目录、存储库、捆绑和插件技能文件夹。", "08a321a984": "调整搜索或筛选条件。", - "4acd6d68ec": "未找到本地技能", + "4acd6d68ec": "未找到技能", "6a62a0168c": "没有匹配项", "cd7893fbc1": "正在扫描技能", "35b9a724a0": "可用的", - "0c74e7ff34": "当地的" + "0c74e7ff34": "已安装" }, "SkillFreshnessNudge": { "titleOne": "已安装的 Orca 技能已过期", diff --git a/src/renderer/src/runtime/runtime-skills-client.test.ts b/src/renderer/src/runtime/runtime-skills-client.test.ts new file mode 100644 index 000000000..197db33cd --- /dev/null +++ b/src/renderer/src/runtime/runtime-skills-client.test.ts @@ -0,0 +1,102 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { SkillDiscoveryResult, SkillDiscoveryTarget } from '../../../shared/skills' +import { + createCompatibleRuntimeStatusResponseIfNeeded, + type RuntimeEnvironmentCallRequest +} from './runtime-compatibility-test-fixture' +import { clearRuntimeCompatibilityCacheForTests } from './runtime-rpc-client' +import { discoverSkillsForRuntimeTarget } from './runtime-skills-client' + +function discoveryResult(skillName: string): SkillDiscoveryResult { + return { + skills: [ + { + id: 'skill-1', + name: skillName, + description: null, + providers: ['agent-skills'], + sourceKind: 'home', + sourceLabel: 'Agent skills home', + rootPath: '/home/dev/.agents/skills', + directoryPath: `/home/dev/.agents/skills/${skillName}`, + skillFilePath: `/home/dev/.agents/skills/${skillName}/SKILL.md`, + installed: true, + fileCount: 1, + updatedAt: null + } + ], + sources: [], + scannedAt: 0 + } +} + +const discover = vi.fn<(target?: SkillDiscoveryTarget) => Promise>() +const runtimeEnvironmentCall = vi.fn() + +beforeEach(() => { + clearRuntimeCompatibilityCacheForTests() + discover.mockReset() + runtimeEnvironmentCall.mockReset() + vi.stubGlobal('window', { + api: { + skills: { discover }, + runtimeEnvironments: { + call: (args: RuntimeEnvironmentCallRequest) => + createCompatibleRuntimeStatusResponseIfNeeded(args) ?? runtimeEnvironmentCall(args) + } + } + }) +}) + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('discoverSkillsForRuntimeTarget', () => { + it('scans the local host through the skills IPC for a local target', async () => { + const result = discoveryResult('orchestration') + discover.mockResolvedValueOnce(result) + const target: SkillDiscoveryTarget = { runtime: 'host' } + + await expect(discoverSkillsForRuntimeTarget({ kind: 'local' }, target)).resolves.toBe(result) + + expect(discover).toHaveBeenCalledWith(target) + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) + + it('routes to the remote runtime and drops the local-only target', async () => { + const result = discoveryResult('orchestration') + runtimeEnvironmentCall.mockResolvedValueOnce({ id: 'skills', ok: true, result }) + + await expect( + discoverSkillsForRuntimeTarget( + { kind: 'environment', environmentId: 'env-1' }, + { runtime: 'wsl', wslDistro: 'Ubuntu' } + ) + ).resolves.toBe(result) + + expect(discover).not.toHaveBeenCalled() + expect(runtimeEnvironmentCall).toHaveBeenCalledWith( + expect.objectContaining({ selector: 'env-1', method: 'skills.discover', params: {} }) + ) + }) + + // Why: no caller can produce these yet, so the remote params must stay empty + // rather than shipping a client-host target the server would misread. + it('sends no target at all to a remote runtime', async () => { + runtimeEnvironmentCall.mockResolvedValueOnce({ + id: 'skills', + ok: true, + result: discoveryResult('orchestration') + }) + + await discoverSkillsForRuntimeTarget( + { kind: 'environment', environmentId: 'env-1' }, + { cwd: '/workspace/app', worktreeId: 'wt-1' } + ) + + expect(runtimeEnvironmentCall).toHaveBeenCalledWith( + expect.objectContaining({ method: 'skills.discover', params: {} }) + ) + }) +}) diff --git a/src/renderer/src/runtime/runtime-skills-client.ts b/src/renderer/src/runtime/runtime-skills-client.ts new file mode 100644 index 000000000..700e90a66 --- /dev/null +++ b/src/renderer/src/runtime/runtime-skills-client.ts @@ -0,0 +1,33 @@ +import type { SkillDiscoveryResult, SkillDiscoveryTarget } from '../../../shared/skills' +import { callRuntimeRpc, type RuntimeClientTarget } from './runtime-rpc-client' + +const SKILL_DISCOVERY_TIMEOUT_MS = 15_000 + +/** + * Discover skills on the runtime that actually runs them: the local desktop host + * (or its WSL/project runtime) by default, or a connected remote Orca runtime + * when one is active. This keeps install badges in sync with where the skill + * files land instead of always reading the client's disk (#6789). + * + * The target is dropped entirely for a remote call. Every target any caller can + * currently produce describes the *client's* host — a WSL distro or a local + * project-runtime resolution — and forwarding those would ask a Linux server to + * resolve a WSL distro it does not have. The server does honour `cwd` and + * `worktreeId` (see `main/runtime/rpc/methods/skills.ts`), so if a caller ever + * supplies workspace identity, forward those two fields rather than widening + * this to the whole target. + */ +export async function discoverSkillsForRuntimeTarget( + runtimeTarget: RuntimeClientTarget, + target?: SkillDiscoveryTarget +): Promise { + if (runtimeTarget.kind === 'local') { + return window.api.skills.discover(target) + } + return callRuntimeRpc( + runtimeTarget, + 'skills.discover', + {}, + { timeoutMs: SKILL_DISCOVERY_TIMEOUT_MS } + ) +} diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index f2275a8b9..8544817d0 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -71,7 +71,7 @@ import { toRuntimeWorktreeSelector } from '../../runtime/runtime-worktree-select import { buildDismissedOnboardingFolderAgentStartup } from '@/lib/onboarding-folder-agent-startup' import { markOnboardingProjectAdded } from '@/lib/onboarding-project-checklist' import { filterSetupScriptPromptDismissalsToValidRepos } from '@/lib/setup-script-prompt' -import { notifyInstalledAgentSkillsChanged } from '@/hooks/useInstalledAgentSkills' +import { notifyInstalledAgentSkillsChanged } from '@/hooks/installed-agent-skill-discovery' import { translate } from '@/i18n/i18n' import { getRepoExecutionHostId, diff --git a/src/renderer/src/store/slices/runtime-status.test.ts b/src/renderer/src/store/slices/runtime-status.test.ts index e3a71e58f..a82350f06 100644 --- a/src/renderer/src/store/slices/runtime-status.test.ts +++ b/src/renderer/src/store/slices/runtime-status.test.ts @@ -373,4 +373,31 @@ describe('runtime-status slice', () => { 'runtime-a' ) }) + + // Why: skill discovery waits for the catalog to settle. A rejected read must + // release that wait without claiming the catalog is hydrated — host routing + // uses `runtimeEnvironmentCatalogHydrated` to fail closed on an unknown + // catalog, and an empty stale list must not be mistaken for "no runtimes". + it('settles but does not hydrate the catalog when the read fails', async () => { + const list = vi.fn().mockRejectedValue(new Error('unreadable environments.json')) + stubRuntimeEnvironmentApi({ getStatus: vi.fn(), list }) + const store = createSliceStore() + + await store.getState().hydrateRuntimeEnvironmentStatuses() + + expect(store.getState().runtimeEnvironmentCatalogSettled).toBe(true) + expect(store.getState().runtimeEnvironmentCatalogHydrated).toBe(false) + expect(store.getState().runtimeEnvironments).toEqual([]) + }) + + it('both settles and hydrates the catalog on a successful read', async () => { + const list = vi.fn().mockResolvedValue([]) + stubRuntimeEnvironmentApi({ getStatus: vi.fn(), list }) + const store = createSliceStore() + + await store.getState().hydrateRuntimeEnvironmentStatuses() + + expect(store.getState().runtimeEnvironmentCatalogSettled).toBe(true) + expect(store.getState().runtimeEnvironmentCatalogHydrated).toBe(true) + }) }) diff --git a/src/renderer/src/store/slices/runtime-status.ts b/src/renderer/src/store/slices/runtime-status.ts index 13386fcd1..03fdac122 100644 --- a/src/renderer/src/store/slices/runtime-status.ts +++ b/src/renderer/src/store/slices/runtime-status.ts @@ -23,8 +23,14 @@ export type RuntimeStatusSlice = { /** Saved remote Orca servers. Host pickers use this to show user-chosen names * instead of opaque runtime ids. */ runtimeEnvironments: PublicKnownRuntimeEnvironment[] - /** True only after the saved-runtime catalog has loaded successfully. */ + /** True only after the saved-runtime catalog has loaded successfully. Gates + * fail-closed host routing, so a failed read must NOT flip it. */ runtimeEnvironmentCatalogHydrated: boolean + /** True once the catalog read has finished, successfully or not. Surfaces that + * only need to stop waiting (skill discovery) read this instead of + * `runtimeEnvironmentCatalogHydrated`, so a failed read degrades rather than + * leaving them pending for the whole session. */ + runtimeEnvironmentCatalogSettled: boolean /** Keyed by runtime environment id. Fed into buildExecutionHostRegistry so * compat verdicts/blocked health show live in the sidebar host pickers. */ runtimeStatusByEnvironmentId: Map @@ -68,6 +74,7 @@ export const createRuntimeStatusSlice: StateCreator ({ runtimeEnvironments: [], runtimeEnvironmentCatalogHydrated: false, + runtimeEnvironmentCatalogSettled: false, runtimeStatusByEnvironmentId: new Map(), removedRuntimeEnvironmentIds: new Set(), @@ -131,6 +138,7 @@ export const createRuntimeStatusSlice: StateCreator { const runtimeEnvironmentCall = vi.fn() const runtimeEnvironmentGetStatus = vi.fn() const settingsSet = vi.fn().mockResolvedValue(undefined) +const settingsGet = vi.fn() +const runtimeEnvironmentList = vi.fn() const setActiveRuntimeEnvironmentPreference = vi.fn().mockResolvedValue(undefined) const worktreesListDetected = vi.fn() @@ -50,6 +52,8 @@ beforeEach(() => { }, _meta: { runtimeId: 'runtime-2' } }) + settingsGet.mockResolvedValue({ notifications: {} }) + runtimeEnvironmentList.mockResolvedValue([]) runtimeEnvironmentCall.mockImplementation( ({ method, params }: { method: string; params?: { repo?: string } }) => { const detectedRepoId = params?.repo ?? 'repo-env-2' @@ -133,8 +137,12 @@ beforeEach(() => { }) vi.stubGlobal('window', { api: { - settings: { set: settingsSet, setActiveRuntimeEnvironmentPreference }, - runtimeEnvironments: { call: runtimeEnvironmentCall, getStatus: runtimeEnvironmentGetStatus }, + settings: { get: settingsGet, set: settingsSet, setActiveRuntimeEnvironmentPreference }, + runtimeEnvironments: { + call: runtimeEnvironmentCall, + getStatus: runtimeEnvironmentGetStatus, + list: runtimeEnvironmentList + }, worktrees: { listDetected: worktreesListDetected } } }) @@ -648,3 +656,29 @@ describe('createSettingsSlice runtime switching', () => { }) }) }) + +describe('fetchSettings runtime catalog probe', () => { + // Why: skill discovery waits for the runtime catalog to settle. If a rejected + // settings read skipped the probe, every skill badge would sit on a spinner + // for the whole session with no retry affordance. + it('still probes the runtime catalog when the settings read fails', async () => { + settingsGet.mockRejectedValueOnce(new Error('unreadable settings.json')) + const store = createTestStore() + + await store.getState().fetchSettings() + await vi.waitFor(() => expect(runtimeEnvironmentList).toHaveBeenCalled()) + + expect(store.getState().settings).toBeNull() + expect(store.getState().runtimeEnvironmentCatalogSettled).toBe(true) + }) + + it('probes the runtime catalog after a successful settings read', async () => { + const store = createTestStore() + + await store.getState().fetchSettings() + await vi.waitFor(() => expect(runtimeEnvironmentList).toHaveBeenCalled()) + + expect(store.getState().settings).not.toBeNull() + expect(store.getState().runtimeEnvironmentCatalogSettled).toBe(true) + }) +}) diff --git a/src/renderer/src/store/slices/settings.ts b/src/renderer/src/store/slices/settings.ts index e0d17d065..7e42263ac 100644 --- a/src/renderer/src/store/slices/settings.ts +++ b/src/renderer/src/store/slices/settings.ts @@ -144,13 +144,14 @@ export const createSettingsSlice: StateCreator try { const settings = await window.api.settings.get() set({ settings }) - // Why: best-effort boot probe so sidebar host pickers show live runtime - // health before the settings pane is ever opened. Fire-and-forget to keep - // startup off the network round-trips. - void get().hydrateRuntimeEnvironmentStatuses() } catch (err) { console.error('Failed to fetch settings:', err) } + // Why: best-effort boot probe so sidebar host pickers show live runtime + // health before the settings pane is ever opened. Fire-and-forget to keep + // startup off the network round-trips. Runs even when settings fail to load, + // so surfaces waiting on the catalog settling are never stranded pending. + void get().hydrateRuntimeEnvironmentStatuses() }, updateSettings: async (updates) => {