From b6143f649701cb24b457db5353b3840bda936afb Mon Sep 17 00:00:00 2001 From: Mega Mind <146339422+M3gA-Mind@users.noreply.github.com> Date: Mon, 29 Jun 2026 09:58:31 +0530 Subject: [PATCH] fix(connections): reflect install progress/state on skill & MCP cards (#4224) --- .../components/skills/SkillsExplorerTab.tsx | 32 +++++++++++++-- .../__tests__/SkillsExplorerTab.test.tsx | 41 +++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/app/src/components/skills/SkillsExplorerTab.tsx b/app/src/components/skills/SkillsExplorerTab.tsx index 19b898c5a..a489f21b1 100644 --- a/app/src/components/skills/SkillsExplorerTab.tsx +++ b/app/src/components/skills/SkillsExplorerTab.tsx @@ -533,6 +533,14 @@ export default function SkillsExplorerTab({ onToast }: SkillsExplorerTabProps) { const [catalogError, setCatalogError] = useState(null); const [catalogInitialized, setCatalogInitialized] = useState(false); const [installingId, setInstallingId] = useState(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>(new Set()); const [sources, setSources] = useState([]); const [activeSources, setActiveSources] = useState>(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) { setDetailEntry(entry)} onInstall={() => void handleRegistryInstall(entry)} @@ -1014,13 +1040,13 @@ export default function SkillsExplorerTab({ onToast }: SkillsExplorerTabProps) { { setDetailEntry(null); setDetailSkill(null); }} onInstall={ - detailEntry && !isCatalogEntryInstalled(detailEntry, installedKeys) + detailEntry && !entryInstalled(detailEntry) ? () => { void handleRegistryInstall(detailEntry); setDetailEntry(null); diff --git a/app/src/components/skills/__tests__/SkillsExplorerTab.test.tsx b/app/src/components/skills/__tests__/SkillsExplorerTab.test.tsx index 380e117d3..a0ac3ee27 100644 --- a/app/src/components/skills/__tests__/SkillsExplorerTab.test.tsx +++ b/app/src/components/skills/__tests__/SkillsExplorerTab.test.tsx @@ -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(); + + 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([]);