fix: resolve active skill across retained slug history (#3405)

This commit is contained in:
Patrick Erichsen
2026-08-04 18:43:46 -07:00
committed by GitHub
parent 2c7c40f001
commit 98a6e04e39
3 changed files with 157 additions and 9 deletions
+32 -7
View File
@@ -12,6 +12,7 @@ import { normalizeSkillSlug } from "../skillSlugValidator";
type DbCtx = Pick<QueryCtx | MutationCtx, "db">;
const MAX_LEGACY_OWNER_MATCHES = 25;
const MAX_PUBLISHER_SLUG_MATCHES = 25;
type LegacyResultQuery<T> = {
take?: (limit: number) => Promise<T[]>;
@@ -97,13 +98,37 @@ export async function getSkillBySlugForPublisher(
slug: string,
publisher: Doc<"publishers">,
) {
const scopedSkill = await ctx.db
.query("skills")
.withIndex("by_owner_publisher_slug", (q) =>
q.eq("ownerPublisherId", publisher._id).eq("slug", slug),
)
.unique();
if (scopedSkill) return scopedSkill;
const scopedCandidates = await takeQueryResults<Doc<"skills">>(
ctx.db
.query("skills")
.withIndex("by_owner_publisher_slug", (q) =>
q.eq("ownerPublisherId", publisher._id).eq("slug", slug),
),
MAX_PUBLISHER_SLUG_MATCHES + 1,
);
if (scopedCandidates.length > MAX_PUBLISHER_SLUG_MATCHES) {
throw new Error(
`Publisher slug history exceeds the safe lookup bound for @${publisher.handle}/${slug}`,
);
}
const activeScopedSkills = scopedCandidates.filter(
(candidate) => candidate.softDeletedAt === undefined,
);
if (activeScopedSkills.length > 1) {
throw new Error(`Active publisher slug invariant violated for @${publisher.handle}/${slug}`);
}
if (activeScopedSkills[0]) return activeScopedSkills[0];
// Retained merge/history rows intentionally share the old owner-scoped slug.
// Keep a single row discoverable for restore/reclaim, but never guess between
// multiple deleted lineages when no active canonical row exists.
const scopedHistory = scopedCandidates;
if (scopedHistory.length > 1) {
throw new Error(
`Soft-deleted publisher slug history is ambiguous for @${publisher.handle}/${slug}`,
);
}
if (scopedHistory[0]) return scopedHistory[0];
const linkedUserId = await getPublisherLegacyOwnerUserId(ctx, publisher);
if (!linkedUserId) return null;
+11 -2
View File
@@ -1765,8 +1765,17 @@ describe("skills ownership", () => {
throw new Error(`unexpected skills index ${name}`);
}
return {
take: async () =>
name === "by_slug" && constraints.slug === "portable" ? [skill] : [],
take: async () => {
if (name === "by_slug" && constraints.slug === "portable") return [skill];
if (
name === "by_owner_publisher_slug" &&
constraints.slug === "portable" &&
constraints.ownerPublisherId === "publishers:org"
) {
return [destinationSkill];
}
return [];
},
unique: async () => {
if (name === "by_slug" && constraints.slug === "portable") return skill;
if (
@@ -0,0 +1,114 @@
/// <reference types="vite/client" />
/* @vitest-environment edge-runtime */
import { convexTest } from "convex-test";
import { expect, it } from "vitest";
import { api, internal } from "./_generated/api";
import schema from "./schema";
const modules = import.meta.glob("./**/*.ts");
async function createPublisherSlugFixture(options: {
activeCount: number;
softDeletedCount: number;
}) {
const t = convexTest(schema, modules);
const ids = await t.run(async (ctx) => {
const userId = await ctx.db.insert("users", { handle: "owner" });
const publisherId = await ctx.db.insert("publishers", {
kind: "user",
handle: "owner",
displayName: "Owner",
linkedUserId: userId,
createdAt: 1,
updatedAt: 1,
});
await ctx.db.patch(userId, { personalPublisherId: publisherId });
const activeSkillIds = [];
for (let index = 0; index < options.activeCount; index += 1) {
activeSkillIds.push(
await ctx.db.insert("skills", {
slug: "same-slug",
displayName: `Active ${index}`,
ownerUserId: userId,
ownerPublisherId: publisherId,
tags: {},
badges: {},
moderationStatus: "active",
stats: { comments: 0, downloads: 0, stars: 0, versions: 0 },
createdAt: index + 1,
updatedAt: index + 1,
}),
);
}
const softDeletedSkillIds = [];
for (let index = 0; index < options.softDeletedCount; index += 1) {
const canonicalSkillId = activeSkillIds[0];
softDeletedSkillIds.push(
await ctx.db.insert("skills", {
slug: "same-slug",
displayName: `History ${index}`,
ownerUserId: userId,
ownerPublisherId: publisherId,
canonicalSkillId,
forkOf: canonicalSkillId
? { skillId: canonicalSkillId, kind: "duplicate", at: 10 + index }
: undefined,
tags: {},
badges: {},
moderationStatus: "hidden",
moderationReason: "owner.merged",
softDeletedAt: 10 + index,
stats: { comments: 0, downloads: 0, stars: 0, versions: 0 },
createdAt: 10 + index,
updatedAt: 10 + index,
}),
);
}
return { activeSkillIds, softDeletedSkillIds };
});
return { t, ...ids };
}
it("resolves the active skill when retained same-publisher history shares its slug", async () => {
const fixture = await createPublisherSlugFixture({ activeCount: 1, softDeletedCount: 2 });
const result = await fixture.t.query(api.skills.getBySlug, {
ownerHandle: "owner",
slug: "same-slug",
});
expect(result?.skill?._id).toBe(fixture.activeSkillIds[0]);
});
it("fails closed when a publisher has multiple active skills with the same slug", async () => {
const fixture = await createPublisherSlugFixture({ activeCount: 2, softDeletedCount: 1 });
await expect(
fixture.t.query(api.skills.getBySlug, { ownerHandle: "owner", slug: "same-slug" }),
).rejects.toThrow(/active publisher slug invariant/i);
});
it("preserves a single soft-deleted skill for restore and reclaim flows", async () => {
const fixture = await createPublisherSlugFixture({ activeCount: 0, softDeletedCount: 1 });
const result = await fixture.t.query(internal.skills.getSkillBySlugIncludingSoftDeletedInternal, {
ownerHandle: "owner",
slug: "same-slug",
});
expect(result?._id).toBe(fixture.softDeletedSkillIds[0]);
});
it("fails closed when only multiple soft-deleted skills share a publisher slug", async () => {
const fixture = await createPublisherSlugFixture({ activeCount: 0, softDeletedCount: 2 });
await expect(
fixture.t.query(internal.skills.getSkillBySlugIncludingSoftDeletedInternal, {
ownerHandle: "owner",
slug: "same-slug",
}),
).rejects.toThrow(/soft-deleted publisher slug history is ambiguous/i);
});