From 28117627e1afdd167b0d9c7c5dfc036391901a75 Mon Sep 17 00:00:00 2001 From: Siddharth Date: Sat, 18 Apr 2026 16:48:30 +0530 Subject: [PATCH] fix: batch TUI bug fixes and type filter feature (#23-#30) - Deduplicate installed entries when same name+path in both scopes (#23) - Skip docs/ directories in community index builder (#24) - Show community vault sources as read-only in Vaults tab (#25) - Warn about unresolvable dependencies during install (#26) - Move integration test output to os.tmpdir() (#27) - Always show scope selector for installs, even single-package (#28) - Hide version badge when version is missing instead of showing 'v?' (#29) - Add type filter toggle (t key) to discover screen (#30) --- plug/src/commands/install.js | 10 +++++- plug/src/tui/components/package-detail.jsx | 10 +++--- plug/src/tui/components/status-line.jsx | 5 +-- plug/src/tui/hooks/use-installed.js | 4 ++- plug/src/tui/hooks/use-packages.js | 2 +- plug/src/tui/hooks/use-vaults.js | 34 +++++++++++++++++++ plug/src/tui/screens/discover.jsx | 25 +++++++++----- plug/src/tui/screens/installed.jsx | 10 +++--- plug/src/tui/screens/vaults.jsx | 19 ++++++----- plug/src/utils/community-index.js | 2 +- plug/tests/community-index.test.js | 8 ++--- .../community-install-integration.test.js | 3 +- plug/tests/tui/discover.test.jsx | 5 +-- 13 files changed, 100 insertions(+), 37 deletions(-) diff --git a/plug/src/commands/install.js b/plug/src/commands/install.js index 70aa471..95b45ae 100644 --- a/plug/src/commands/install.js +++ b/plug/src/commands/install.js @@ -163,6 +163,7 @@ export async function runInstall(name, options = {}) { const pkgSpecMap = new Map(); pkgSpecMap.set(pkgName, { pkg, vault, rawBaseUrl: pkg.rawBaseUrl ?? null }); + const unresolvedDeps = []; for (const depName of installOrder) { if (depName === pkgName || pkgSpecMap.has(depName)) continue; try { @@ -170,11 +171,18 @@ export async function runInstall(name, options = {}) { if (depMatches.length > 0) { const { pkg: depPkg, vault: depVault } = depMatches[0]; pkgSpecMap.set(depName, { pkg: depPkg, vault: depVault, rawBaseUrl: depPkg.rawBaseUrl ?? null }); + } else { + unresolvedDeps.push(depName); + verbose(`Dependency '${depName}' not found in any vault — skipping`); } } catch { - // Skip unresolvable deps silently + unresolvedDeps.push(depName); + verbose(`Dependency '${depName}' lookup failed — skipping`); } } + if (unresolvedDeps.length > 0 && !ctx.json) { + console.log(chalk.yellow(`⚠ Could not resolve ${unresolvedDeps.length} dependenc${unresolvedDeps.length === 1 ? 'y' : 'ies'}: ${unresolvedDeps.join(', ')}`)); + } // Root's direct dependency names (for tracking) const rootDirectDeps = (pkg.dependencies ?? []).map(d => (typeof d === 'string' ? d : d.name)); diff --git a/plug/src/tui/components/package-detail.jsx b/plug/src/tui/components/package-detail.jsx index 31cc716..ace88c7 100644 --- a/plug/src/tui/components/package-detail.jsx +++ b/plug/src/tui/components/package-detail.jsx @@ -47,10 +47,12 @@ export default function PackageDetail({ pkg, onBack, onInstall, installedNames = {/* Metadata */} - - Version : - {pkg.version || '?'} - + {pkg.version && ( + + Version : + {pkg.version} + + )} Vault : {pkg.vault} diff --git a/plug/src/tui/components/status-line.jsx b/plug/src/tui/components/status-line.jsx index 2dade5e..8392c32 100644 --- a/plug/src/tui/components/status-line.jsx +++ b/plug/src/tui/components/status-line.jsx @@ -13,15 +13,16 @@ import { Box, Text } from 'ink'; * searchFocused: boolean, * }} props */ -export default function StatusLine({ cursor, total, filtered, isFiltered, searchFocused }) { +export default function StatusLine({ cursor, total, filtered, isFiltered, searchFocused, typeFilter = 'all' }) { const position = total > 0 ? `${cursor + 1}/${filtered}` : '0/0'; const filterNote = isFiltered ? ` (filtered from ${total})` : ''; const searchHint = searchFocused ? ' · type to filter · Esc to clear' : ''; + const typeLabel = typeFilter !== 'all' ? ` · [${typeFilter}s]` : ''; return ( - Discover plugins {position}{filterNote}{searchHint} + Discover plugins {position}{filterNote}{typeLabel}{searchHint} ); diff --git a/plug/src/tui/hooks/use-installed.js b/plug/src/tui/hooks/use-installed.js index 838ba99..178c4ab 100644 --- a/plug/src/tui/hooks/use-installed.js +++ b/plug/src/tui/hooks/use-installed.js @@ -45,12 +45,14 @@ export function useInstalled() { ]); const merged = []; + const localPaths = new Map(); for (const [name, record] of Object.entries(localData.installed || {})) { merged.push({ name, ...record, scope: 'local', hasUpdate: false, latestVersion: null }); + localPaths.set(name, record.path); } for (const [name, record] of Object.entries(globalData.installed || {})) { - // Deduplicate: if same name in both scopes, keep both (different scope label) + if (localPaths.has(name) && localPaths.get(name) === record.path) continue; merged.push({ name, ...record, scope: 'global', hasUpdate: false, latestVersion: null }); } diff --git a/plug/src/tui/hooks/use-packages.js b/plug/src/tui/hooks/use-packages.js index 67e6f38..5f6abd0 100644 --- a/plug/src/tui/hooks/use-packages.js +++ b/plug/src/tui/hooks/use-packages.js @@ -101,7 +101,7 @@ function addVaultPackages(list, vaultName, registry) { list.push({ name, vault: vaultName, - version: pkg.version || '?', + version: pkg.version || null, type: pkg.type || 'skill', description: pkg.description || '', tags: pkg.tags || [], diff --git a/plug/src/tui/hooks/use-vaults.js b/plug/src/tui/hooks/use-vaults.js index b934277..55492f0 100644 --- a/plug/src/tui/hooks/use-vaults.js +++ b/plug/src/tui/hooks/use-vaults.js @@ -1,6 +1,7 @@ import { useState, useEffect, useCallback } from 'react'; import { getConfig } from '../../utils/config.js'; import { getCachedRegistry } from '../../utils/registry.js'; +import { getCachedCommunityIndex, getStaleCommunityIndexCache } from '../../utils/community-index.js'; /** * Loads vault configuration and enriches each vault with metadata: @@ -68,6 +69,39 @@ export function useVaults() { }); } + const communityData = await getCachedCommunityIndex() || await getStaleCommunityIndexCache(); + if (communityData) { + const pkgs = communityData.packages || []; + const communityVaultMap = new Map(); + for (const pkg of pkgs) { + if (!pkg.vault || enriched.some(v => v.name === pkg.vault)) continue; + if (!communityVaultMap.has(pkg.vault)) { + communityVaultMap.set(pkg.vault, { count: 0, url: pkg.vaultUrl || '' }); + } + communityVaultMap.get(pkg.vault).count++; + } + for (const [name, info] of communityVaultMap) { + let owner = ''; + let repo = ''; + try { + const parts = new URL(info.url).pathname.split('/').filter(Boolean); + owner = parts[0] || ''; + repo = parts[1] || ''; + } catch {} + enriched.push({ + name, + owner, + repo, + branch: 'main', + private: false, + isDefault: false, + githubUrl: info.url, + packageCount: info.count, + isCommunity: true, + }); + } + } + if (!cancelled) { setVaults(enriched); setLoading(false); diff --git a/plug/src/tui/screens/discover.jsx b/plug/src/tui/screens/discover.jsx index eb24f12..862db95 100644 --- a/plug/src/tui/screens/discover.jsx +++ b/plug/src/tui/screens/discover.jsx @@ -38,10 +38,16 @@ export default function DiscoverScreen({ onInputCapture }) { const [installedNames, setInstalledNames] = useState(new Set()); const [depPlan, setDepPlan] = useState(null); const [planLoading, setPlanLoading] = useState(false); + const [typeFilter, setTypeFilter] = useState('all'); const installingRef = useRef(false); + const TYPE_FILTERS = ['all', 'skill', 'agent', 'command']; + // Filtered/searched package list - const filteredPackages = useSearch(packages, searchQuery); + const searchedPackages = useSearch(packages, searchQuery); + const filteredPackages = typeFilter === 'all' + ? searchedPackages + : searchedPackages.filter(p => p.type === typeFilter); // Load installed package names useEffect(() => { @@ -76,6 +82,14 @@ export default function DiscoverScreen({ onInputCapture }) { return; } + if (input === 't') { + setTypeFilter((prev) => { + const idx = TYPE_FILTERS.indexOf(prev); + return TYPE_FILTERS[(idx + 1) % TYPE_FILTERS.length]; + }); + return; + } + // Action keys only fire when search is unfocused if (input === 'i' && filteredPackages.length > 0) { const queue = toggled.size > 0 @@ -148,13 +162,7 @@ export default function DiscoverScreen({ onInputCapture }) { .then((plan) => { setPlanLoading(false); setDepPlan(plan); - // Single package (no extra deps) → skip plan screen, install immediately - if (plan.toInstall.length <= 1) { - setView('installing'); - installingRef.current = true; - doInstall(queue).finally(() => { installingRef.current = false; }); - } - // else: stay on plan view so user can confirm + // Always show plan screen so user can choose project vs global scope }) .catch(() => { // Resolver failed — fall back to direct install @@ -327,6 +335,7 @@ export default function DiscoverScreen({ onInputCapture }) { filtered={filteredPackages.length} isFiltered={isFiltered} searchFocused={searchFocused} + typeFilter={typeFilter} /> ); diff --git a/plug/src/tui/screens/installed.jsx b/plug/src/tui/screens/installed.jsx index 54d9e44..7d0da63 100644 --- a/plug/src/tui/screens/installed.jsx +++ b/plug/src/tui/screens/installed.jsx @@ -231,10 +231,12 @@ function InstalledDetail({ pkg, onBack, onUpdate, onRemove }) { - - Version : - {pkg.version || '?'} - + {pkg.version && ( + + Version : + {pkg.version} + + )} Vault : {pkg.vault || '?'} diff --git a/plug/src/tui/screens/vaults.jsx b/plug/src/tui/screens/vaults.jsx index 5a48fbd..cff91f1 100644 --- a/plug/src/tui/screens/vaults.jsx +++ b/plug/src/tui/screens/vaults.jsx @@ -55,7 +55,7 @@ export default function VaultsScreen({ onInputCapture }) { if (input === 'r' && vaults.length > 0) { const vault = vaults[cursor]; - if (vault) { + if (vault && !vault.isCommunity) { setPendingRemove(vault); setView('confirm-remove'); } @@ -63,7 +63,7 @@ export default function VaultsScreen({ onInputCapture }) { if (input === 'd' && vaults.length > 0) { const vault = vaults[cursor]; - if (vault && !vault.isDefault) { + if (vault && !vault.isDefault && !vault.isCommunity) { doSetDefault(vault.name); } } @@ -277,10 +277,9 @@ function VaultList({ vaults, cursor, terminalWidth }) { function VaultItem({ vault, isCursor, terminalWidth }) { const star = vault.isDefault ? '★' : ' '; const cursorStr = isCursor ? '>' : ' '; - const privLabel = vault.private ? '[private]' : '[public] '; + const privLabel = vault.isCommunity ? '[community]' : vault.private ? '[private]' : '[public] '; const pkgStr = vault.packageCount !== null ? `${vault.packageCount} pkgs` : '? pkgs'; - // Line 1: cursor star name · owner/repo · branch · visibility · pkg count const line1 = `${vault.name} · ${vault.owner}/${vault.repo} · ${vault.branch} · ${privLabel} · ${pkgStr}`; const prefixLen = 4; // " > " const maxLine1 = terminalWidth - prefixLen - 2; @@ -320,11 +319,11 @@ function VaultStatusLine({ cursor, total }) { // ── ConfirmRemoveVault ──────────────────────────────────────────────────────── function ConfirmRemoveVault({ vault, onConfirm, onCancel }) { - const isOfficial = vault.name === 'official'; + const isProtected = vault.name === 'official' || vault.isCommunity; useInput((input, key) => { if (key.escape || input === 'n' || input === 'N') onCancel(); - if (!isOfficial && (input === 'y' || input === 'Y')) onConfirm(); + if (!isProtected && (input === 'y' || input === 'Y')) onConfirm(); }); return ( @@ -333,10 +332,14 @@ function ConfirmRemoveVault({ vault, onConfirm, onCancel }) { Remove Vault - {isOfficial ? ( + {isProtected ? ( <> - Cannot remove the official vault — it is required for plug to function. + + {vault.isCommunity + ? 'Cannot remove community vault sources — they are read-only.' + : 'Cannot remove the official vault — it is required for plug to function.'} + Press Esc or [n] to go back diff --git a/plug/src/utils/community-index.js b/plug/src/utils/community-index.js index b910a40..ff68c05 100644 --- a/plug/src/utils/community-index.js +++ b/plug/src/utils/community-index.js @@ -88,7 +88,7 @@ export function normalizeCommunityPackage(pkg) { name: pkg.name, vault: pkg.vault, vaultUrl: pkg.vaultUrl, - version: pkg.version ?? '?', + version: pkg.version || null, type: pkg.type, description: pkg.description ?? '', tags: pkg.tags ?? [], diff --git a/plug/tests/community-index.test.js b/plug/tests/community-index.test.js index fbb8601..51fcdc2 100644 --- a/plug/tests/community-index.test.js +++ b/plug/tests/community-index.test.js @@ -183,15 +183,15 @@ describe('community-index utils', () => { expect(result.path).toBe(base.directory); }); - it('version defaults to "?" when null', () => { + it('version defaults to null when null', () => { const result = normalizeCommunityPackage({ ...base, version: null }); - expect(result.version).toBe('?'); + expect(result.version).toBeNull(); }); - it('version defaults to "?" when undefined', () => { + it('version defaults to null when undefined', () => { const { version: _v, ...noVersion } = base; const result = normalizeCommunityPackage(noVersion); - expect(result.version).toBe('?'); + expect(result.version).toBeNull(); }); it('tags defaults to [] when null', () => { diff --git a/plug/tests/community-install-integration.test.js b/plug/tests/community-install-integration.test.js index 0e5d843..0de2f64 100644 --- a/plug/tests/community-install-integration.test.js +++ b/plug/tests/community-install-integration.test.js @@ -20,6 +20,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import path from 'path'; +import os from 'os'; import fs from 'fs/promises'; import { installSinglePackage } from '../src/commands/install.js'; import { fetchCommunityIndex, normalizeCommunityPackage } from '../src/utils/community-index.js'; @@ -28,7 +29,7 @@ import { ctx } from '../src/utils/context.js'; const LOCAL_INDEX = process.env.LOCAL_INDEX || ''; const OUTPUT_DIR = process.env.OUTPUT_DIR - || path.resolve(import.meta.dirname, '..', 'test-output', 'community-install'); + || path.join(os.tmpdir(), 'plug-test-output', 'community-install'); const LIMIT = process.env.LIMIT ? parseInt(process.env.LIMIT, 10) : Infinity; const FILTER = process.env.FILTER || ''; diff --git a/plug/tests/tui/discover.test.jsx b/plug/tests/tui/discover.test.jsx index 02b744a..5c6a97d 100644 --- a/plug/tests/tui/discover.test.jsx +++ b/plug/tests/tui/discover.test.jsx @@ -244,9 +244,10 @@ describe('DiscoverScreen', () => { stdin.write('i'); await new Promise((r) => setTimeout(r, 200)); - // Should go directly to installing view (not plan view) + // Should now show plan view with scope selector even for single package const frame = lastFrame(); - expect(frame).not.toContain('Will install'); + expect(frame).toContain('Will install'); + expect(frame).toContain('Project'); }); it('Tab key toggles scope between Project and Global in plan view', async () => {