fix(connections): reflect install progress/state on skill & MCP cards (#4224)

This commit is contained in:
Mega Mind
2026-06-28 21:28:31 -07:00
committed by GitHub
parent 90d574917d
commit b6143f6497
2 changed files with 70 additions and 3 deletions
@@ -533,6 +533,14 @@ export default function SkillsExplorerTab({ onToast }: SkillsExplorerTabProps) {
const [catalogError, setCatalogError] = useState<string | null>(null);
const [catalogInitialized, setCatalogInitialized] = useState(false);
const [installingId, setInstallingId] = useState<string | null>(null);
// Catalog entry ids we just installed this session. The "installed" badge is
// otherwise derived purely from `isCatalogEntryInstalled`, a heuristic that
// maps a refetched installed skill (whose post-install id/location can differ
// from the catalog entry) back to the catalog card. When that mapping misses,
// a successful install fell back to "Install" — the only signal was a fleeting
// toast, so the card looked unchanged (#4150). Recording the installed entry
// id here makes the card flip to "Installed" deterministically on success.
const [installedEntryIds, setInstalledEntryIds] = useState<Set<string>>(new Set());
const [sources, setSources] = useState<string[]>([]);
const [activeSources, setActiveSources] = useState<Set<string>>(new Set());
@@ -638,6 +646,16 @@ export default function SkillsExplorerTab({ onToast }: SkillsExplorerTabProps) {
[skills]
);
// A catalog entry counts as installed if the refetched installed list maps
// back to it (`isCatalogEntryInstalled`) OR we installed it this session. The
// latter guarantees the card reflects a successful install even when the
// heuristic key-match misses (#4150).
const entryInstalled = useCallback(
(entry: CatalogEntry): boolean =>
installedEntryIds.has(entry.id) || isCatalogEntryInstalled(entry, installedKeys),
[installedEntryIds, installedKeys]
);
const filteredSkills = useMemo(() => {
const q = searchQuery.toLowerCase().trim();
if (!q) return skills;
@@ -701,6 +719,14 @@ export default function SkillsExplorerTab({ onToast }: SkillsExplorerTabProps) {
setInstallingId(entry.id);
try {
const result = await skillRegistryApi.install(entry.id);
// Authoritatively mark this entry installed so the card flips to
// "Installed" on success regardless of whether the refetched list maps
// back to it via the install-key heuristic (#4150).
setInstalledEntryIds(prev => {
const next = new Set(prev);
next.add(entry.id);
return next;
});
// Await the refetch so `installedKeys` is fresh before the button
// re-renders — otherwise it briefly flips back to "Install" between
// clearing the installing state and the list updating. `fetchSkills`
@@ -975,7 +1001,7 @@ export default function SkillsExplorerTab({ onToast }: SkillsExplorerTabProps) {
<CatalogTile
key={`${entry.source}-${entry.id}`}
entry={entry}
installed={isCatalogEntryInstalled(entry, installedKeys)}
installed={entryInstalled(entry)}
installing={installingId === entry.id}
onClick={() => setDetailEntry(entry)}
onInstall={() => void handleRegistryInstall(entry)}
@@ -1014,13 +1040,13 @@ export default function SkillsExplorerTab({ onToast }: SkillsExplorerTabProps) {
<SkillDetailDialog
entry={detailEntry}
skill={detailSkill}
installed={detailEntry ? isCatalogEntryInstalled(detailEntry, installedKeys) : true}
installed={detailEntry ? entryInstalled(detailEntry) : true}
onClose={() => {
setDetailEntry(null);
setDetailSkill(null);
}}
onInstall={
detailEntry && !isCatalogEntryInstalled(detailEntry, installedKeys)
detailEntry && !entryInstalled(detailEntry)
? () => {
void handleRegistryInstall(detailEntry);
setDetailEntry(null);
@@ -358,6 +358,47 @@ describe('SkillsExplorerTab', () => {
expect(within(tile).getByTestId('registry-install-built-in/apple-notes')).toBeInTheDocument();
});
// #4150: a successful install must flip the card to "Installed" even when the
// refetched installed list does NOT map back to the catalog entry via the
// install-key heuristic — otherwise the card reverted to "Install" and the
// only signal of success was a fleeting toast.
it('marks a catalog entry installed on success even when the refetched list does not map back', async () => {
const { workflowsApi } = await import('../../../services/api/workflowsApi');
const { skillRegistryApi } = await import('../../../services/api/skillRegistryApi');
const catalogEntry = {
...MOCK_CATALOG_ENTRY,
id: 'built-in/apple-notes',
name: 'Apple Notes',
docs_path: 'skills/apple-notes/SKILL.md',
};
// The installed list never resolves to anything that maps back to the entry
// (simulates a post-install id/location the heuristic can't match).
vi.mocked(workflowsApi.listWorkflows).mockResolvedValue([]);
vi.mocked(skillRegistryApi.browse).mockResolvedValue([catalogEntry]);
vi.mocked(skillRegistryApi.install).mockResolvedValue({
url: '',
stdout: '',
stderr: '',
newSkills: ['apple-notes'],
});
render(<SkillsExplorerTab />);
const installBtn = await screen.findByTestId('registry-install-built-in/apple-notes');
await act(async () => {
fireEvent.click(installBtn);
});
const tile = screen.getByTestId('registry-tile-built-in/apple-notes');
await waitFor(() => {
expect(within(tile).getByText('Installed')).toBeInTheDocument();
});
expect(skillRegistryApi.install).toHaveBeenCalledWith('built-in/apple-notes');
expect(
within(tile).queryByTestId('registry-install-built-in/apple-notes')
).not.toBeInTheDocument();
});
it('has an install from URL button', async () => {
const { workflowsApi } = await import('../../../services/api/workflowsApi');
vi.mocked(workflowsApi.listWorkflows).mockResolvedValue([]);