diff --git a/crates/hk-core/src/store.rs b/crates/hk-core/src/store.rs index 89b9714..a942670 100644 --- a/crates/hk-core/src/store.rs +++ b/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 = 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 = 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 = 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(); diff --git a/src/lib/types.ts b/src/lib/types.ts index 507b403..1b04cd0 100644 --- a/src/lib/types.ts +++ b/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. */ diff --git a/src/stores/__tests__/extension-helpers.test.ts b/src/stores/__tests__/extension-helpers.test.ts index 3b44531..58a10ad 100644 --- a/src/stores/__tests__/extension-helpers.test.ts +++ b/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" }; diff --git a/src/stores/extension-helpers.ts b/src/stores/extension-helpers.ts index c05bd2b..16e7fbc 100644 --- a/src/stores/extension-helpers.ts +++ 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>(); + 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(); + keys.add(extensionGroupKey(ext)); + urlSiblings.set(sk, keys); + } + const map = new Map(); 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]); diff --git a/src/stores/project-store.ts b/src/stores/project-store.ts index 9807be2..4cf204c 100644 --- a/src/stores/project-store.ts +++ b/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((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((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(); }, }));