fix(1.3.1): grouping dedup + web project refresh + delete cascade

crates/hk-core/src/store.rs

@@ -1,6 +1,6 @@
 use crate::HkError;
 use chrono::{DateTime, Utc};
-use rusqlite::{params, Connection};
+use rusqlite::{params, Connection, OptionalExtension};
 use std::path::{Path, PathBuf};
 
 use crate::models::*;
@@ -771,6 +771,19 @@ impl Store {
             Self::sync_extension_agents(&tx, &ext.id, &ext.agents)?;
         }
 
+        // Drop any extension rows scoped to a project that no longer exists.
+        // delete_project now cascades, but pre-1.3.1 deletions left orphans
+        // behind that the stale-cleanup below would preserve (because they
+        // carry install_meta from a marketplace install). Self-heal here so
+        // upgrading users don't have to manually clear them.
+        tx.execute(
+            "DELETE FROM extensions \
+             WHERE json_extract(scope_json, '$.type') = 'project' \
+               AND json_extract(scope_json, '$.path') NOT IN \
+                   (SELECT path FROM projects)",
+            [],
+        )?;
+
         // Remove stale extensions no longer on disk — but keep:
         // - Disabled ones (intentionally absent from scan results)
         // - Extensions with install_meta (user explicitly installed, e.g. CLI via install_cli)
@@ -1071,6 +1084,27 @@ impl Store {
     }
 
     pub fn delete_project(&self, id: &str) -> Result<(), HkError> {
+        // Look up the project's path before deletion so we can cascade-delete
+        // any extensions scoped to it. Without this, scope_json continues to
+        // reference a project that no longer exists in the projects table,
+        // and those rows show up as ghosts in the "All scopes" view with no
+        // project to filter into.
+        let path: Option<String> = self
+            .conn
+            .query_row(
+                "SELECT path FROM projects WHERE id = ?1",
+                params![id],
+                |row| row.get::<_, String>(0),
+            )
+            .optional()?;
+        if let Some(path) = path {
+            self.conn.execute(
+                "DELETE FROM extensions \
+                 WHERE json_extract(scope_json, '$.type') = 'project' \
+                   AND json_extract(scope_json, '$.path') = ?1",
+                params![path],
+            )?;
+        }
         self.conn
             .execute("DELETE FROM projects WHERE id = ?1", params![id])?;
         Ok(())
@@ -1503,6 +1537,93 @@ mod tests {
         assert!(projects.is_empty());
     }
 
+    #[test]
+    fn test_delete_project_cascades_to_extensions() {
+        let (store, _dir) = test_store();
+        let project = Project {
+            id: "proj-001".into(),
+            name: "my-project".into(),
+            path: "/tmp/my-project".into(),
+            created_at: Utc::now(),
+            exists: true,
+        };
+        store.insert_project(&project).unwrap();
+
+        // One extension in the project, one global, one in a different project.
+        // Only the first should disappear when proj-001 is deleted.
+        let mut in_project = sample_extension();
+        in_project.id = "ext-in-project".into();
+        in_project.scope = ConfigScope::Project {
+            name: "my-project".into(),
+            path: "/tmp/my-project".into(),
+        };
+        store.insert_extension(&in_project).unwrap();
+
+        let mut global = sample_extension();
+        global.id = "ext-global".into();
+        store.insert_extension(&global).unwrap();
+
+        let other = Project {
+            id: "proj-002".into(),
+            name: "other".into(),
+            path: "/tmp/other".into(),
+            created_at: Utc::now(),
+            exists: true,
+        };
+        store.insert_project(&other).unwrap();
+        let mut in_other = sample_extension();
+        in_other.id = "ext-in-other".into();
+        in_other.scope = ConfigScope::Project {
+            name: "other".into(),
+            path: "/tmp/other".into(),
+        };
+        store.insert_extension(&in_other).unwrap();
+
+        store.delete_project("proj-001").unwrap();
+
+        let remaining: Vec<String> = store
+            .list_extensions(None, None)
+            .unwrap()
+            .into_iter()
+            .map(|e| e.id)
+            .collect();
+        assert!(!remaining.contains(&"ext-in-project".to_string()));
+        assert!(remaining.contains(&"ext-global".to_string()));
+        assert!(remaining.contains(&"ext-in-other".to_string()));
+    }
+
+    #[test]
+    fn test_sync_extensions_purges_orphan_project_rows() {
+        // Pre-1.3.1 delete_project did not cascade, leaving extension rows
+        // pointing at a project that no longer existed. Simulate that state
+        // by inserting an extension scoped to a project we never inserted,
+        // then verify the next sync_extensions clears it out.
+        let (store, _dir) = test_store();
+
+        let mut orphan = sample_extension();
+        orphan.id = "ext-orphan".into();
+        orphan.scope = ConfigScope::Project {
+            name: "ghost".into(),
+            path: "/tmp/ghost".into(),
+        };
+        store.insert_extension(&orphan).unwrap();
+
+        let mut keep = sample_extension();
+        keep.id = "ext-keep".into();
+        store.insert_extension(&keep).unwrap();
+
+        store.sync_extensions(&[keep.clone()]).unwrap();
+
+        let remaining: Vec<String> = store
+            .list_extensions(None, None)
+            .unwrap()
+            .into_iter()
+            .map(|e| e.id)
+            .collect();
+        assert!(!remaining.contains(&"ext-orphan".to_string()));
+        assert!(remaining.contains(&"ext-keep".to_string()));
+    }
+
     #[test]
     fn test_find_siblings_by_source_path() {
         let (store, _dir) = test_store();

src/lib/types.ts

@@ -103,34 +103,43 @@ function extractDeveloper(url: string | null): string {
  *  install time). Fall back to it so the 6 marketplace copies of the same
  *  skill group together and stay separate from a same-named hand-written
  *  project skill (which has neither field set). */
-export function extensionGroupKey(ext: Extension): string {
-  let name = ext.name;
+/** Logical name used for grouping. For hooks the wire name is
+ *  `event:matcher:command`; we group by command only so the same command
+ *  deployed to agents with different events merges into one row. */
+export function logicalExtensionName(ext: Extension): string {
   if (ext.kind === "hook") {
-    // name format is "event:matcher:command" — extract just the command part
-    const parts = name.split(":");
-    if (parts.length >= 3) {
-      name = parts.slice(2).join(":");
-    }
+    const parts = ext.name.split(":");
+    if (parts.length >= 3) return parts.slice(2).join(":");
   }
-  // Resolution order: source.url → install_meta.url → pack (synthesized to
-  // a github URL so extractDeveloper handles it uniformly). `pack` is a
-  // user-editable field on the detail panel; treating it as a tiebreaker
-  // means a user can merge two unlinked rows into one group by typing the
-  // owner/repo identifier (e.g. arxiv-search where only one of four
-  // copies carries install_meta from the original install).
-  const url =
+  return ext.name;
+}
+
+/** Authoritative "where did this come from" URL for grouping purposes.
+ *  Resolution order: source.url → install_meta.url → pack (synthesized to a
+ *  GitHub URL so extractDeveloper handles it uniformly). `pack` is a
+ *  user-editable field on the detail panel; treating it as a tiebreaker
+ *  means a user can merge two unlinked rows into one group by typing the
+ *  owner/repo identifier (e.g. arxiv-search where only one of four copies
+ *  carries install_meta from the original install). Returns `null` when an
+ *  extension is truly sourceless (hand-written project skill, agent-bundled
+ *  global skill the user never linked, etc.). */
+export function deriveExtensionUrl(ext: Extension): string | null {
+  return (
     ext.source.url ??
     ext.install_meta?.url ??
-    (ext.pack ? `https://github.com/${ext.pack}` : null);
-  // When everything else is null (truly sourceless, e.g. a hand-written
-  // project skill or an agent-bundled global skill the user never linked),
-  // fall back to scopeKey so a project-level "code-review" doesn't
-  // accidentally merge with an unrelated global "code-review" of the same
-  // name. A future install-to-project of a marketplace skill will set
-  // install_meta and the URL branch above wins, so it correctly merges
-  // with same-source siblings in other scopes.
+    (ext.pack ? `https://github.com/${ext.pack}` : null)
+  );
+}
+
+export function extensionGroupKey(ext: Extension): string {
+  // When the URL is null, fall back to scopeKey so a project-level
+  // "code-review" doesn't accidentally merge with an unrelated global
+  // "code-review" of the same name. A future install-to-project of a
+  // marketplace skill will set install_meta and the URL branch above wins,
+  // so it correctly merges with same-source siblings in other scopes.
+  const url = deriveExtensionUrl(ext);
   const developer = url ? extractDeveloper(url) : `(${scopeKey(ext.scope)})`;
-  return `${ext.kind}\0${name}\0${developer}`;
+  return `${ext.kind}\0${logicalExtensionName(ext)}\0${developer}`;
 }
 
 /** Sort agent name strings by canonical display order. */

src/stores/__tests__/extension-helpers.test.ts

@@ -49,6 +49,43 @@ describe("buildGroups", () => {
     expect(groups[0].agents).toContain("cursor");
   });
 
+  it("merges a sourceless row into a URL-based sibling sharing kind+name+scope", () => {
+    // When some instances of the same logical extension carry pack/url
+    // metadata and others don't (e.g. a later scan finds a copy without
+    // marketplace provenance), they should still group into one row.
+    const shared: Extension = {
+      ...baseExt,
+      source: { origin: "agent", url: null, version: null, commit_hash: null },
+      install_meta: null,
+    };
+    const withPack = { ...shared, pack: "owner/repo" };
+    const a = { ...withPack, id: "a", agents: ["x"] };
+    const b = { ...withPack, id: "b", agents: ["y"] };
+    const c = { ...shared, id: "c", agents: ["z"], pack: null };
+
+    const groups = buildGroups([a, b, c]);
+
+    expect(groups).toHaveLength(1);
+    expect(groups[0].instances).toHaveLength(3);
+  });
+
+  it("does NOT merge a sourceless row when there are multiple URL-based siblings (ambiguous)", () => {
+    const shared: Extension = {
+      ...baseExt,
+      source: { origin: "agent", url: null, version: null, commit_hash: null },
+      install_meta: null,
+    };
+    const a = { ...shared, id: "a", agents: ["x"], pack: "owner-1/repo" };
+    const b = { ...shared, id: "b", agents: ["y"], pack: "owner-2/repo" };
+    const c = { ...shared, id: "c", agents: ["z"], pack: null };
+
+    const groups = buildGroups([a, b, c]);
+
+    // Two distinct URL-based developers → can't decide where `c` belongs;
+    // it stays as its own group rather than getting attached arbitrarily.
+    expect(groups).toHaveLength(3);
+  });
+
   it("separates extensions with different names", () => {
     const a = { ...baseExt, id: "a", name: "skill-a" };
     const b = { ...baseExt, id: "b", name: "skill-b" };

src/stores/extension-helpers.ts

@@ -1,5 +1,11 @@
 import type { Extension, ExtensionKind, GroupedExtension } from "@/lib/types";
-import { extensionGroupKey, sortAgentNames } from "@/lib/types";
+import {
+  deriveExtensionUrl,
+  extensionGroupKey,
+  logicalExtensionName,
+  scopeKey,
+  sortAgentNames,
+} from "@/lib/types";
 import type { ScopeValue } from "@/stores/scope-store";
 
 // ---------------------------------------------------------------------------
@@ -52,9 +58,31 @@ function deduplicatePermissions(
 }
 
 export function buildGroups(extensions: Extension[]): GroupedExtension[] {
+  // Pre-pass: index URL-keyed groups by (kind, logical name, scope) so a
+  // sourceless instance (e.g. an agent-discovered copy that lacks pack
+  // metadata) can attach to its marketplace-installed sibling instead of
+  // forming a separate row. Only redirect when there is exactly one such
+  // sibling — multiple distinct developers in the same scope means we can't
+  // tell which one a sourceless row belongs to.
+  const urlSiblings = new Map<string, Set<string>>();
+  for (const ext of extensions) {
+    if (deriveExtensionUrl(ext) == null) continue;
+    const sk = `${ext.kind}\0${logicalExtensionName(ext)}\0${scopeKey(ext.scope)}`;
+    const keys = urlSiblings.get(sk) ?? new Set<string>();
+    keys.add(extensionGroupKey(ext));
+    urlSiblings.set(sk, keys);
+  }
+
   const map = new Map<string, Extension[]>();
   for (const ext of extensions) {
-    const key = extensionGroupKey(ext);
+    let key = extensionGroupKey(ext);
+    if (deriveExtensionUrl(ext) == null) {
+      const sk = `${ext.kind}\0${logicalExtensionName(ext)}\0${scopeKey(ext.scope)}`;
+      const siblings = urlSiblings.get(sk);
+      if (siblings?.size === 1) {
+        key = siblings.values().next().value as string;
+      }
+    }
     const list = map.get(key);
     if (list) list.push(ext);
     else map.set(key, [ext]);

src/stores/project-store.ts

@@ -1,6 +1,7 @@
 import { create } from "zustand";
 import { api } from "@/lib/invoke";
 import type { Project } from "@/lib/types";
+import { useExtensionStore } from "./extension-store";
 import { useScopeStore } from "./scope-store";
 import { toast } from "./toast-store";
 
@@ -33,6 +34,16 @@ export const useProjectStore = create<ProjectState>((set, get) => ({
   async addProject(path: string) {
     const project = await api.addProject(path);
     set((s) => ({ projects: [...s.projects, project] }));
+    // Discover the new project's extensions and refresh the in-memory
+    // list. Without this, web-mode users see no extensions for the
+    // newly-added project until they refresh the page (desktop relies on
+    // the Tauri `extensions-changed` event, which has no web equivalent).
+    try {
+      await api.scanAndSync();
+    } catch (e) {
+      console.error("Failed to scan after adding project:", e);
+    }
+    await useExtensionStore.getState().fetch();
   },
 
   async removeProject(id: string) {
@@ -48,5 +59,9 @@ export const useProjectStore = create<ProjectState>((set, get) => ({
         );
       }
     }
+    // Backend cascades the project's extension rows on delete, so refresh
+    // the in-memory list to drop the now-stale entries (web mode has no
+    // event channel for this; see addProject above).
+    await useExtensionStore.getState().fetch();
   },
 }));