feat: add ability to delete version tags from skill detail page (#1380)

* feat: add ability to delete version tags from skill detail page

- Add deleteTags mutation to convex/skills.ts (protects 'latest' tag)
- Add delete button (×) on each tag in SkillHeader (visible to owner/moderator only)
- Wire up onTagDelete prop from SkillDetailPage to SkillHeader
- Add .tag-delete CSS styles

Closes: version tags accumulate across publishes with no way to remove them

* fix: address review feedback on deleteTags PR

- Add window.confirm() before deleting a tag (P2: missing confirmation)
- Skip db.patch when no tags are actually removed (P2: unnecessary write)
- Add test suite for deleteTags mutation covering:
  - Tag deletion with latest protection
  - No-op when only latest is targeted
  - No-op for nonexistent tags
  - Permission check for non-owner
  - Moderator access on other user's skill
  - Skill not found error

* fix: repair deleteTags test harness

* fix: satisfy deleteTags test typecheck

---------

Co-authored-by: Jeff <tjefferson518@gmail.com>

convex/skills.deleteTags.test.ts

@@ -0,0 +1,179 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+vi.mock("@convex-dev/auth/server", () => ({
+  getAuthUserId: vi.fn(),
+  authTables: {},
+}));
+
+const { getAuthUserId } = await import("@convex-dev/auth/server");
+const { deleteTags } = await import("./skills");
+
+type WrappedHandler<TArgs, TResult = unknown> = {
+  _handler: (ctx: unknown, args: TArgs) => Promise<TResult>;
+};
+
+const deleteTagsHandler = (
+  deleteTags as unknown as WrappedHandler<{
+    skillId: string;
+    tags: string[];
+  }>
+)._handler;
+
+function buildGlobalStatsQuery(table: string) {
+  if (table !== "globalStats") return null;
+  return {
+    withIndex: () => ({
+      unique: async () => ({ _id: "globalStats:1", activeSkillsCount: 100 }),
+    }),
+  };
+}
+
+function buildDigestQuery(table: string) {
+  if (table !== "skillSearchDigest") return null;
+  return {
+    withIndex: () => ({
+      unique: async () => null,
+    }),
+  };
+}
+
+function makeCtx(params: {
+  user: Record<string, unknown>;
+  skill: Record<string, unknown> | null;
+}) {
+  vi.mocked(getAuthUserId).mockResolvedValue(params.user._id as never);
+  const patch = vi.fn(async (_id: string, value: Record<string, unknown>) => value);
+  const db = {
+    get: vi.fn(async (id: string) => {
+      if (id === params.user._id) return params.user;
+      if (params.skill && id === params.skill._id) return params.skill;
+      return null;
+    }),
+    query: vi.fn((table: string) => {
+      const globalStatsQuery = buildGlobalStatsQuery(table);
+      if (globalStatsQuery) return globalStatsQuery;
+      const digestQuery = buildDigestQuery(table);
+      if (digestQuery) return digestQuery;
+      throw new Error(`unexpected table ${table}`);
+    }),
+    insert: vi.fn(),
+    patch,
+    delete: vi.fn(),
+    replace: vi.fn(),
+    normalizeId: vi.fn(() => null),
+  };
+  const auth = { getUserIdentity: vi.fn(async () => ({ tokenIdentifier: "test" })) };
+  return { db, auth, patch };
+}
+
+const ownerUser = {
+  _id: "users:owner",
+  deletedAt: undefined,
+  deactivatedAt: undefined,
+  role: undefined,
+};
+
+const modUser = {
+  _id: "users:mod",
+  deletedAt: undefined,
+  deactivatedAt: undefined,
+  role: "moderator",
+};
+
+const otherUser = {
+  _id: "users:other",
+  deletedAt: undefined,
+  deactivatedAt: undefined,
+  role: undefined,
+};
+
+const baseSkill = {
+  _id: "skills:1",
+  ownerUserId: "users:owner",
+  tags: {
+    latest: "versions:3",
+    stable: "versions:2",
+    beta: "versions:3",
+    "old-tag": "versions:1",
+  },
+  moderationStatus: "active",
+  moderationFlags: undefined,
+  softDeletedAt: undefined,
+};
+
+describe("deleteTags", () => {
+  beforeEach(() => {
+    vi.mocked(getAuthUserId).mockReset();
+  });
+
+  it("deletes specified tags and keeps latest", async () => {
+    const { db, auth, patch } = makeCtx({ user: ownerUser, skill: baseSkill });
+    await deleteTagsHandler(
+      { db, auth } as never,
+      { skillId: "skills:1", tags: ["stable", "old-tag"] } as never,
+    );
+    expect(patch).toHaveBeenCalledOnce();
+    const patchArgs = patch.mock.calls[0];
+    expect(patchArgs[1]).toHaveProperty("tags");
+    const newTags = (patchArgs[1] as Record<string, unknown>).tags as Record<string, string>;
+    expect(newTags).toHaveProperty("latest");
+    expect(newTags).toHaveProperty("beta");
+    expect(newTags).not.toHaveProperty("stable");
+    expect(newTags).not.toHaveProperty("old-tag");
+  });
+
+  it("protects the latest tag from deletion", async () => {
+    const { db, auth, patch } = makeCtx({ user: ownerUser, skill: baseSkill });
+    await deleteTagsHandler(
+      { db, auth } as never,
+      { skillId: "skills:1", tags: ["latest"] } as never,
+    );
+    // No actual tag removed → no db.patch call
+    expect(patch).not.toHaveBeenCalled();
+  });
+
+  it("skips db write when no tags are actually removed", async () => {
+    const { db, auth, patch } = makeCtx({ user: ownerUser, skill: baseSkill });
+    await deleteTagsHandler(
+      { db, auth } as never,
+      { skillId: "skills:1", tags: ["nonexistent", "latest"] } as never,
+    );
+    expect(patch).not.toHaveBeenCalled();
+  });
+
+  it("throws for non-owner non-moderator user", async () => {
+    const { db, auth } = makeCtx({ user: otherUser, skill: baseSkill });
+    await expect(
+      deleteTagsHandler(
+        { db, auth } as never,
+        { skillId: "skills:1", tags: ["stable"] } as never,
+      ),
+    ).rejects.toThrow();
+  });
+
+  it("allows moderator to delete tags on other user's skill", async () => {
+    const { db, auth, patch } = makeCtx({ user: modUser, skill: baseSkill });
+    await deleteTagsHandler(
+      { db, auth } as never,
+      { skillId: "skills:1", tags: ["beta"] } as never,
+    );
+    expect(patch).toHaveBeenCalledOnce();
+    const newTags = (patch.mock.calls[0][1] as Record<string, unknown>).tags as Record<
+      string,
+      string
+    >;
+    expect(newTags).not.toHaveProperty("beta");
+    expect(newTags).toHaveProperty("latest");
+    expect(newTags).toHaveProperty("stable");
+  });
+
+  it("throws when skill not found", async () => {
+    const { db, auth } = makeCtx({ user: ownerUser, skill: null });
+    await expect(
+      deleteTagsHandler(
+        { db, auth } as never,
+        { skillId: "skills:missing", tags: ["stable"] } as never,
+      ),
+    ).rejects.toThrow("Skill not found");
+  });
+});

convex/skills.ts

@@ -4764,6 +4764,38 @@ export const updateTags = mutation({
   },
 });
 
+export const deleteTags = mutation({
+  args: {
+    skillId: v.id("skills"),
+    tags: v.array(v.string()),
+  },
+  handler: async (ctx, args) => {
+    const { user } = await requireUser(ctx);
+    const skill = await ctx.db.get(args.skillId);
+    if (!skill) throw new Error("Skill not found");
+    if (skill.ownerUserId !== user._id) {
+      assertModerator(user);
+    }
+
+    const nextTags = { ...skill.tags };
+    let changed = false;
+    for (const tag of args.tags) {
+      if (tag === "latest") continue; // protect the latest tag from deletion
+      if (tag in nextTags) {
+        delete nextTags[tag];
+        changed = true;
+      }
+    }
+
+    if (!changed) return;
+
+    await ctx.db.patch(skill._id, {
+      tags: nextTags,
+      updatedAt: Date.now(),
+    });
+  },
+});
+
 export const setRedactionApproved = mutation({
   args: { skillId: v.id("skills"), approved: v.boolean() },
   handler: async (ctx, args) => {

src/components/SkillDetailPage.tsx

@@ -80,6 +80,7 @@ export function SkillDetailPage({
   const toggleStar = useMutation(api.stars.toggle);
   const reportSkill = useMutation(api.skills.report);
   const updateTags = useMutation(api.skills.updateTags);
+  const deleteTags = useMutation(api.skills.deleteTags);
   const getReadme = useAction(api.skills.getReadme);
   const myPublishers = useQuery(api.publishers.listMine) as
     | Array<{ publisher: { _id: Id<"publishers"> }; role: string }>
@@ -289,6 +290,15 @@ export function SkillDetailPage({
     });
   };
 
+  const deleteTag = (tag: string) => {
+    if (!skill) return;
+    if (!window.confirm(`Delete tag "${tag}"?`)) return;
+    void deleteTags({
+      skillId: skill._id,
+      tags: [tag],
+    });
+  };
+
   const submitReport = async () => {
     if (!skill) return;
 
@@ -372,6 +382,7 @@ export function SkillDetailPage({
           tagVersionId={tagVersionId}
           onTagVersionChange={setTagVersionId}
           onTagSubmit={submitTag}
+          onTagDelete={deleteTag}
           tagVersions={versions ?? []}
           clawdis={clawdis}
           osLabels={osLabels}

src/components/SkillHeader.tsx

@@ -71,6 +71,7 @@ type SkillHeaderProps = {
   tagVersionId: Id<"skillVersions"> | "";
   onTagVersionChange: (value: Id<"skillVersions"> | "") => void;
   onTagSubmit: () => void;
+  onTagDelete: (tag: string) => void;
   tagVersions: Doc<"skillVersions">[];
   clawdis: ClawdisSkillMetadata | undefined;
   osLabels: string[];
@@ -110,6 +111,7 @@ export function SkillHeader({
   tagVersionId,
   onTagVersionChange,
   onTagSubmit,
+  onTagDelete,
   tagVersions,
   clawdis,
   osLabels,
@@ -364,6 +366,17 @@ export function SkillHeader({
                 <span className="tag-meta">
                   v{versionById.get(versionId)?.version ?? versionId}
                 </span>
+                {canManage && tag !== "latest" ? (
+                  <button
+                    type="button"
+                    className="tag-delete"
+                    onClick={() => onTagDelete(tag)}
+                    aria-label={`Delete tag ${tag}`}
+                    title={`Delete tag "${tag}"`}
+                  >
+                    ×
+                  </button>
+                ) : null}
               </span>
             ))
           )}

src/styles.css

@@ -2341,6 +2341,22 @@ code {
   opacity: 0.7;
 }
 
+.tag-delete {
+  all: unset;
+  cursor: pointer;
+  font-size: 0.85rem;
+  line-height: 1;
+  opacity: 0.5;
+  padding: 0 2px;
+  margin-left: -2px;
+  border-radius: 3px;
+  transition: opacity 0.15s;
+}
+
+.tag-delete:hover {
+  opacity: 1;
+}
+
 .tag-form {
   display: grid;
   grid-template-columns: minmax(140px, 1fr) minmax(160px, 1fr) auto;