Merge pull request #87 from laststance/remove-delete-btn

fix(marketplace): remove uninstall button + harden migration & CLI parser (v0.11.2)

Author: ryota-murakami

SPEC.md

@@ -94,7 +94,6 @@ GUI wrapper for `npx skills` CLI commands:
 - [x] Install skills via `npx skills add <repo>`
 - [x] Select target agents for installation
 - [x] Installation progress tracking
-- [ ] Remove skills via `npx skills remove <name>`
 
 ### Symlink Status
 
@@ -146,14 +145,14 @@ Each skill displays:
 
 ### Actions
 
-| Action                 | Status     | Notes                            |
-| ---------------------- | ---------- | -------------------------------- |
-| View skill details     | ✅ Done    | -                                |
-| View symlink status    | ✅ Done    | -                                |
-| Search skills          | ✅ Done    | Marketplace tab                  |
-| Install skill          | ✅ Done    | With agent selection             |
-| Remove skill           | 🚧 Planned | UI exists, backend not connected |
-| Repair broken symlinks | 🚧 Planned | -                                |
+| Action                 | Status     | Notes                                                  |
+| ---------------------- | ---------- | ------------------------------------------------------ |
+| View skill details     | ✅ Done    | -                                                      |
+| View symlink status    | ✅ Done    | -                                                      |
+| Search skills          | ✅ Done    | Marketplace tab                                        |
+| Install skill          | ✅ Done    | With agent selection                                   |
+| Uninstall skill        | ✅ CLI     | `npx skills remove <name> --global` (no in-app button) |
+| Repair broken symlinks | 🚧 Planned | -                                                      |
 
 ## Tech Stack
 
@@ -357,7 +356,6 @@ listenerMiddleware.startListening({
 // Skills CLI (Marketplace)
 'skills:cli:search'   → Promise<SkillSearchResult[]>
 'skills:cli:install'  → Promise<CliCommandResult>
-'skills:cli:remove'   → Promise<CliCommandResult>
 'skills:cli:cancel'   → void
 'skills:cli:progress' → (Main → Renderer event)
 ```
@@ -442,12 +440,7 @@ interface InstallProgress {
   percent?: number
 }
 
-type MarketplaceStatus =
-  | 'idle'
-  | 'searching'
-  | 'installing'
-  | 'removing'
-  | 'error'
+type MarketplaceStatus = 'idle' | 'searching' | 'installing' | 'error'
 ```
 
 ## Redux State
@@ -488,7 +481,6 @@ interface MarketplaceState {
   searchResults: SkillSearchResult[]
   selectedSkill: SkillSearchResult | null
   installProgress: InstallProgress | null
-  skillToRemove: string | null
   error: string | null
 }
 ```
@@ -543,8 +535,7 @@ skills-desktop/
 │   │       │   │   ├── SkillsMarketplace.tsx
 │   │       │   │   ├── MarketplaceSearch.tsx
 │   │       │   │   ├── SkillRowMarketplace.tsx
-│   │       │   │   ├── InstallModal.tsx
-│   │       │   │   └── RemoveDialog.tsx
+│   │       │   │   └── InstallModal.tsx
 │   │       │   └── ui/             # shadcn/ui components
 │   │       ├── views/
 │   │       ├── hooks/
@@ -659,11 +650,10 @@ APPLE_KEYCHAIN_PROFILE=skills-desktop pnpm build:mac
 
 The Marketplace feature wraps `npx skills@<SKILLS_CLI_VERSION>` CLI commands (version pinned in `src/shared/constants.ts`):
 
-| Feature | CLI Command                                     | Options                          |
-| ------- | ----------------------------------------------- | -------------------------------- |
-| Search  | `npx skills@<SKILLS_CLI_VERSION> find <query>`  | -                                |
-| Install | `npx skills@<SKILLS_CLI_VERSION> add <repo>`    | `-y`, `-g`, `--agent`, `--skill` |
-| Remove  | `npx skills@<SKILLS_CLI_VERSION> remove <name>` | -                                |
+| Feature | CLI Command                                    | Options                          |
+| ------- | ---------------------------------------------- | -------------------------------- |
+| Search  | `npx skills@<SKILLS_CLI_VERSION> find <query>` | -                                |
+| Install | `npx skills@<SKILLS_CLI_VERSION> add <repo>`   | `-y`, `-g`, `--agent`, `--skill` |
 
 **CLI Output Parsing:**
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "skills-desktop",
-  "version": "0.11.1",
+  "version": "0.11.2",
   "description": "Visualize installed Skills and symlink status across AI agents",
   "main": "./out/main/index.mjs",
   "author": "Laststance.io",

src/main/ipc/ipc-schemas.ts

@@ -65,7 +65,6 @@ export const IPC_ARG_SCHEMAS: Partial<Record<IpcInvokeChannel, z.ZodTuple>> = {
 
   // CLI operations
   'skills:cli:search': z.tuple([z.string()]),
-  'skills:cli:remove': z.tuple([nonEmptyString]),
   'skills:cli:install': z.tuple([
     z.object({
       repo: nonEmptyString,

src/main/ipc/skillsCli.ts

@@ -32,10 +32,6 @@ export function registerSkillsCliHandlers(): void {
     }
   })
 
-  typedHandle(IPC_CHANNELS.SKILLS_CLI_REMOVE, async (_, skillName) => {
-    return skillsCliService.remove(skillName)
-  })
-
   typedHandle(IPC_CHANNELS.SKILLS_CLI_CANCEL, () => {
     skillsCliService.cancel()
   })

src/main/services/leaderboardService.ts

@@ -1,5 +1,6 @@
 import { repositoryId } from '../../shared/types'
 import type { RankingFilter, SkillSearchResult } from '../../shared/types'
+import { REPO_PATTERN, SKILL_NAME_PATTERN } from '../utils/skillIdentifiers'
 
 /** Maps ranking filter to skills.sh URL */
 const LEADERBOARD_URLS: Record<RankingFilter, string> = {
@@ -84,6 +85,13 @@ export function parseLeaderboardHtml(html: string): SkillSearchResult[] {
 
     const name = h3Match[1].trim()
 
+    // Defense-in-depth: skills.sh HTML is upstream-controlled. Without this
+    // whitelist, a malformed `<h3>` could land in the installed-badge
+    // aria-label/title (`npx skills remove <name> --global`) and become a
+    // copy-paste hazard. Mirrors `parseSearchOutput`'s CLI-side guard so both
+    // ingestion paths enforce the same identifier contract.
+    if (!REPO_PATTERN.test(repo) || !SKILL_NAME_PATTERN.test(name)) continue
+
     // Extract install count: look for numbers with optional K/M/B suffix.
     // Excludes rank numbers (preceded by #) and delta numbers (preceded by +/-)
     // This appears as standalone text like "731.2K", "20.0K", "927"

src/main/services/skillsCliService.ts

@@ -11,6 +11,7 @@ import type {
   CliCommandResult,
   InstallProgress,
 } from '../../shared/types'
+import { REPO_PATTERN, SKILL_NAME_PATTERN } from '../utils/skillIdentifiers'
 
 /**
  * Build agent ID to CLI name mapping from AGENT_DEFINITIONS
@@ -93,19 +94,6 @@ class SkillsCliService extends EventEmitter {
     return result
   }
 
-  /**
-   * Remove a skill using `npx skills remove <name> -g -y`
-   * @param skillName - Name of the skill to remove
-   * @returns CLI command result
-   * @example
-   * remove('vercel-react-best-practices')
-   */
-  async remove(skillName: string): Promise<CliCommandResult> {
-    // -g flag for global scope (skills are installed globally)
-    // -y flag skips interactive confirmation prompts
-    return this.execCli(['remove', skillName, '-g', '-y'])
-  }
-
   /**
    * Cancel the current CLI operation
    */
@@ -193,6 +181,12 @@ class SkillsCliService extends EventEmitter {
       const match = line.match(/^([^@\s]+)@([^\s]+)$/)
       if (match) {
         const [, repo, name] = match
+        // Reject anything that isn't a plain npm/GitHub identifier — see
+        // SKILL_NAME_PATTERN comment. Without this, a malformed CLI line
+        // could land in aria-labels and copy-paste hints downstream.
+        if (!REPO_PATTERN.test(repo) || !SKILL_NAME_PATTERN.test(name)) {
+          continue
+        }
         // Next line should be the URL
         const urlLine = lines[i + 1]?.trim()
         const urlMatch = urlLine?.match(/^[└├]\s*(https?:\/\/[^\s]+)$/)

src/main/utils/skillIdentifiers.ts

@@ -0,0 +1,20 @@
+/**
+ * Whitelist patterns for skill identifiers parsed from external sources.
+ * Both `parseSearchOutput` (skills CLI text) and `parseLeaderboardHtml`
+ * (skills.sh HTML) feed names into UI strings — aria-labels, titles,
+ * copy-paste hints. A malformed upstream value could carry shell
+ * metacharacters, whitespace, or `..` traversal-shaped tokens that
+ * surface as broken UI or, worse, get pasted into a terminal verbatim.
+ *
+ * Each segment must:
+ *  - start with an alphanumeric (rejects `.`, `..`, `.git`, `-x`)
+ *  - contain only `[a-zA-Z0-9._-]` thereafter
+ *
+ * Sharing one source means both parsers stay in lockstep — bumping
+ * the rule in one place can't silently leave the other parser laxer.
+ */
+const SEGMENT = /[a-zA-Z0-9][a-zA-Z0-9._-]*/
+export const SKILL_NAME_PATTERN = new RegExp(`^${SEGMENT.source}$`)
+export const REPO_PATTERN = new RegExp(
+  `^${SEGMENT.source}\\/${SEGMENT.source}$`,
+)

src/preload/index.ts

@@ -16,7 +16,6 @@ import type {
   RemoveAllFromAgentOptions,
   RestoreDeletedSkillOptions,
   SearchQuery,
-  SkillName,
   SyncExecuteOptions,
   UnlinkFromAgentOptions,
   UnlinkManyFromAgentOptions,
@@ -99,8 +98,6 @@ contextBridge.exposeInMainWorld('electron', {
       typedInvoke('skills:cli:search', query),
     install: async (options: InstallOptions) =>
       typedInvoke('skills:cli:install', options),
-    remove: async (skillName: SkillName) =>
-      typedInvoke('skills:cli:remove', skillName),
     cancel: async () => typedInvoke('skills:cli:cancel'),
     onProgress: createIpcListener<InstallProgress>(
       IPC_CHANNELS.SKILLS_CLI_PROGRESS,

src/renderer/src/components/dashboard/utils/gridConstants.ts

@@ -3,12 +3,18 @@
  *
  * The detail panel is narrow (often 300–500px), so a 6-column grid gives enough
  * resolution for two small widgets side-by-side without forcing each widget
- * into a sliver. Row height is intentionally small (48px) so widgets can
- * express their height in rounded increments (2=header+counter, 3=small list,
- * 4+=scrollable list).
+ * into a sliver.
+ *
+ * Row height is 64px. `WidgetShell` burns a fixed 36px header on every widget
+ * (see WidgetShell.tsx — `h-9` Tailwind class; keep both sites in sync if the
+ * header height ever changes). A 48px row left h=2 widgets with only ~68px of
+ * body — not enough for stat tiles (icon+number+label ≈ 72px) or the health
+ * widget's label/bar/legend stack (≈90px). Bumping to 64 gives h=2 ≈ 100px
+ * body (fits the tightest content + breathing room) and keeps h=3/h=4 generous
+ * for lists and cards.
  */
 export const GRID_COLS = 6
-export const GRID_ROW_HEIGHT_PX = 48
+export const GRID_ROW_HEIGHT_PX = 64
 export const GRID_MARGIN_PX: readonly [number, number] = [8, 8]
 export const GRID_CONTAINER_PADDING_PX: readonly [number, number] = [12, 12]
 

src/renderer/src/components/dashboard/utils/widgetPresets.ts

@@ -44,7 +44,7 @@ const PAGE_SPECS: readonly PageSpec[] = [
   },
   {
     name: 'Actions',
-    widgets: [{ type: 'quick-actions', x: 0, y: 0, w: 6, h: 2 }],
+    widgets: [{ type: 'quick-actions', x: 0, y: 0, w: 6, h: 3 }],
   },
   {
     name: 'Personal',