From a46fb60eee74c0d42a19361a5a6117c95eeefba0 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 15:56:05 +0100 Subject: [PATCH 1/3] feat(core): works catalogue and matching Co-Authored-By: Claude Sonnet 5.5 --- packages/core/src/matching.ts | 166 ++++++++++++++++ packages/core/src/works.ts | 294 ++++++++++++++++++++++++++++ packages/core/test/matching.test.ts | 102 ++++++++++ packages/core/test/works.test.ts | 112 +++++++++++ 4 files changed, 674 insertions(+) create mode 100644 packages/core/src/matching.ts create mode 100644 packages/core/src/works.ts create mode 100644 packages/core/test/matching.test.ts create mode 100644 packages/core/test/works.test.ts diff --git a/packages/core/src/matching.ts b/packages/core/src/matching.ts new file mode 100644 index 0000000..ddc0563 --- /dev/null +++ b/packages/core/src/matching.ts @@ -0,0 +1,166 @@ +import { sql } from "kysely"; +import type { WorkHint } from "@sttracker/plugin-sdk/types"; +import type { Db } from "@sttracker/db/db"; +import { newId, nowIso } from "@sttracker/db/ids"; +import { + createWorkIn, + findWorkByExternalIdsIn, + normalizePosition, + resolveAliasIn, + toWork, + updateWorkFromHintIn, + type K, +} from "./works.ts"; + +export interface FuzzyInput { + title: string; + year?: number | null; + creators?: { name: string }[]; +} + +function normalize(s: string): string { + return s + .normalize("NFD") + .replace(/\p{M}+/gu, "") + .toLowerCase() + .replace(/[^\p{L}\p{N}]+/gu, " ") + .trim() + .replace(/^(the|a|an)\s+/, ""); +} + +function tokens(s: string): string[] { + return normalize(s).split(" ").filter(Boolean); +} + +/** + * 0..1 similarity. Title token-set Jaccard weighs 0.7; year 0.2 (±1 year counts half); any shared + * creator 0.1. A component missing on either side is left out and the weights renormalised, so a + * title-only comparison can still reach 1. + */ +export function fuzzyScore(a: FuzzyInput, b: FuzzyInput): number { + const ta = new Set(tokens(a.title)); + const tb = new Set(tokens(b.title)); + let inter = 0; + for (const t of ta) if (tb.has(t)) inter++; + const union = ta.size + tb.size - inter; + const jaccard = union === 0 ? 0 : inter / union; + let score = jaccard * 0.7; + let weight = 0.7; + if (a.year != null && b.year != null) { + const d = Math.abs(a.year - b.year); + score += d === 0 ? 0.2 : d === 1 ? 0.1 : 0; + weight += 0.2; + } + const ca = new Set((a.creators ?? []).map((c) => normalize(c.name)).filter(Boolean)); + const cb = (b.creators ?? []).map((c) => normalize(c.name)).filter(Boolean); + if (ca.size > 0 && cb.length > 0) { + if (cb.some((n) => ca.has(n))) score += 0.1; + weight += 0.1; + } + return score / weight; +} + +interface Resolved { + workId: string; + created: boolean; + draft: boolean; +} + +async function resolve(k: K, userId: string | null, hint: WorkHint, actor: string): Promise { + // 1. Any external ID. + const byId = await findWorkByExternalIdsIn(k, hint.externalIds); + if (byId) { + await updateWorkFromHintIn(k, byId, hint, actor); + const owner = await k.selectFrom("works").select("owner_user_id").where("id", "=", byId).executeTakeFirstOrThrow(); + const workId = userId ? await resolveAliasIn(k, userId, byId) : byId; + return { workId, created: false, draft: owner.owner_user_id !== null }; + } + + // 2. Parent + position. + const parentId = hint.parent ? (await resolve(k, null, hint.parent, actor)).workId : null; + const position = normalizePosition(hint.position); + if (parentId && position) { + const child = await k + .selectFrom("works") + .select("id") + .where("parent_id", "=", parentId) + .where("position", "=", position) + .where("merged_into_id", "is", null) + .where("owner_user_id", "is", null) + .executeTakeFirst(); + if (child) { + await updateWorkFromHintIn(k, child.id, hint, actor); + return { workId: child.id, created: false, draft: false }; + } + return { workId: await createWorkIn(k, { hint, parentId }, actor), created: true, draft: false }; + } + + // 3. External IDs but no match: new shared work. + if (Object.keys(hint.externalIds).length > 0) { + return { workId: await createWorkIn(k, { hint, parentId }, actor), created: true, draft: false }; + } + + if (userId === null) { + // Parent of a shared child: reuse an identical shared work, else create one. + let q = k + .selectFrom("works") + .select("id") + .where("owner_user_id", "is", null) + .where("merged_into_id", "is", null) + .where("media_type", "=", hint.mediaType) + .where(sql`lower(title) = ${hint.title.toLowerCase()}`); + q = hint.year === undefined ? q : q.where("year", "=", hint.year); + q = parentId ? q.where("parent_id", "=", parentId) : q.where("parent_id", "is", null); + const same = await q.executeTakeFirst(); + if (same) return { workId: same.id, created: false, draft: false }; + return { workId: await createWorkIn(k, { hint, parentId }, actor), created: true, draft: false }; + } + + // 4. Private draft plus fuzzy suggestions. + const workId = await createWorkIn(k, { hint, ownerUserId: userId }, actor); + await suggest(k, userId, workId, hint); + return { workId, created: true, draft: true }; +} + +async function suggest(k: K, userId: string, draftId: string, hint: WorkHint): Promise { + // Bound the candidate set with LIKE on the significant title tokens (OR), so "The Expanse: Leviathan + // Wakes" still finds "Leviathan Wakes". Tokens of 3+ chars, first five; falls back to the first token. + const all = tokens(hint.title); + const sig = all.filter((t) => t.length >= 3).slice(0, 5); + const likeTokens = sig.length > 0 ? sig : all.slice(0, 1); + if (likeTokens.length === 0) return; + const rows = await k + .selectFrom("works") + .selectAll() + .where("merged_into_id", "is", null) + .where("id", "!=", draftId) + .where((eb) => + eb.or([ + eb.and([eb("owner_user_id", "is", null), eb("media_type", "=", hint.mediaType)]), + eb("owner_user_id", "=", userId), + ]), + ) + .where((eb) => + eb.or(likeTokens.map((t) => eb(sql`lower(title)`, "like", `%${t}%`))), + ) + .limit(200) + .execute(); + const scored = rows + .map(toWork) + .map((w) => ({ id: w.id, score: fuzzyScore(hint, w) })) + .filter((c) => c.score >= 0.6) + .sort((a, b) => b.score - a.score) + .slice(0, 5); + const now = nowIso(); + for (const c of scored) { + await k + .insertInto("match_suggestions") + .values({ id: newId(), user_id: userId, work_id: draftId, candidate_work_id: c.id, score: c.score, status: "pending", created_at: now }) + .execute(); + } +} + +/** Resolve a hint to a work per spec §8.4. Never auto-links on fuzzy matches; those become suggestions. */ +export function matchWork(db: Db, userId: string, hint: WorkHint, actor: string): Promise { + return db.k.transaction().execute((tx) => resolve(tx, userId, hint, actor)); +} diff --git a/packages/core/src/works.ts b/packages/core/src/works.ts new file mode 100644 index 0000000..2e9ccca --- /dev/null +++ b/packages/core/src/works.ts @@ -0,0 +1,294 @@ +import { sql, type Kysely } from "kysely"; +import type { MediaType, WorkHint } from "@sttracker/plugin-sdk/types"; +import type { Db } from "@sttracker/db/db"; +import { newId, nowIso } from "@sttracker/db/ids"; +import { fromJson, toJson } from "@sttracker/db/json"; +import type { Database, WorksTable } from "@sttracker/db/schema"; +import { AuthError, NotFoundError, ValidationError } from "./errors.ts"; + +export type Creator = { name: string; role?: string }; +export type Position = { season?: number; episode?: number; index?: number }; + +/** A `works` row with JSON columns parsed. Other fields keep their column names. */ +export interface Work extends Omit { + creators: Creator[]; + metadata: Record; + position: Position | null; +} + +/** Any Kysely handle: the root one or a transaction. */ +export type K = Kysely; + +/** Normalised JSON for `works.position`: keys in a fixed order (season, episode, index), or null when empty. */ +export function normalizePosition(p: Position | undefined | null): string | null { + if (!p) return null; + const o: Position = {}; + if (p.season !== undefined) o.season = p.season; + if (p.episode !== undefined) o.episode = p.episode; + if (p.index !== undefined) o.index = p.index; + return Object.keys(o).length === 0 ? null : toJson(o); +} + +export function toWork(r: WorksTable): Work { + return { + ...r, + creators: fromJson(r.creators, []), + metadata: fromJson>(r.metadata, {}), + position: fromJson(r.position, null), + }; +} + +export async function createWorkIn( + k: K, + input: { hint: WorkHint; ownerUserId?: string | null; parentId?: string | null }, + actor: string, +): Promise { + const { hint } = input; + if (hint.title.trim() === "") throw new ValidationError("title", "required"); + const id = newId(); + const now = nowIso(); + await k + .insertInto("works") + .values({ + id, + media_type: hint.mediaType, + title: hint.title, + original_title: "", + year: hint.year ?? null, + release_date: null, + creators: toJson(hint.creators ?? []), + description: hint.description ?? "", + cover_url: hint.coverUrl ?? "", + cover_attachment_id: null, + metadata: "{}", + parent_id: input.parentId ?? null, + position: normalizePosition(hint.position), + owner_user_id: input.ownerUserId ?? null, + merged_into_id: null, + created_at: now, + updated_at: now, + }) + .execute(); + for (const [namespace, value] of Object.entries(hint.externalIds)) { + await k.insertInto("external_ids").values({ id: newId(), work_id: id, namespace, value }).execute(); + } + await k + .insertInto("work_history") + .values({ id: newId(), work_id: id, before: "{}", after: toJson({ created: true, title: hint.title }), actor, created_at: now }) + .execute(); + return id; +} + +/** Insert the work and its external IDs. */ +export function createWork( + db: Db, + input: { hint: WorkHint; ownerUserId?: string | null; parentId?: string | null }, + actor: string, +): Promise { + return db.k.transaction().execute((tx) => createWorkIn(tx, input, actor)); +} + +export async function getWorkIn(k: K, id: string): Promise { + const seen = new Set(); + let cur: string | null = id; + while (cur && !seen.has(cur)) { + seen.add(cur); + const row: WorksTable | undefined = await k.selectFrom("works").selectAll().where("id", "=", cur).executeTakeFirst(); + if (!row) return null; + if (!row.merged_into_id) return toWork(row); + cur = row.merged_into_id; + } + return null; +} + +/** Fetch a work, following `merged_into_id` tombstones to the surviving one. */ +export function getWork(db: Db, id: string): Promise { + return getWorkIn(db.k, id); +} + +/** Follow `merged_into_id` to the surviving id (the input id if it is not merged). */ +export async function finalWorkId(k: K, id: string): Promise { + return (await getWorkIn(k, id))?.id ?? id; +} + +export async function findWorkByExternalIdsIn(k: K, ids: Record): Promise { + for (const [namespace, value] of Object.entries(ids)) { + const row = await k + .selectFrom("external_ids") + .select("work_id") + .where("namespace", "=", namespace) + .where("value", "=", value) + .executeTakeFirst(); + if (row) return finalWorkId(k, row.work_id); + } + return null; +} + +/** First work matching any (namespace, value) pair, following merges. */ +export function findWorkByExternalIds(db: Db, ids: Record): Promise { + return findWorkByExternalIdsIn(db.k, ids); +} + +async function addMissingExternalIds(k: K, workId: string, ids: Record): Promise { + const added: string[] = []; + for (const [namespace, value] of Object.entries(ids)) { + const exists = await k + .selectFrom("external_ids") + .select("id") + .where("namespace", "=", namespace) + .where("value", "=", value) + .executeTakeFirst(); + if (exists) continue; + await k.insertInto("external_ids").values({ id: newId(), work_id: workId, namespace, value }).execute(); + added.push(`${namespace}:${value}`); + } + return added; +} + +export async function updateWorkFromHintIn(k: K, workId: string, hint: WorkHint, actor: string): Promise { + const row = await k.selectFrom("works").selectAll().where("id", "=", workId).executeTakeFirst(); + if (!row || row.owner_user_id !== null) return; // private works are never touched by syncs + const before: Record = {}; + const after: Record = {}; + const patch: Partial = {}; + const set = (field: F, next: WorksTable[F] | undefined, empty: boolean) => { + if (next === undefined || empty || next === row[field]) return; + before[field] = row[field]; + after[field] = next; + patch[field] = next; + }; + set("title", hint.title, hint.title.trim() === ""); + set("year", hint.year, hint.year === undefined); + set("creators", hint.creators ? toJson(hint.creators) : undefined, !hint.creators || hint.creators.length === 0); + set("description", hint.description, !hint.description); + set("cover_url", hint.coverUrl, !hint.coverUrl); + const addedIds = await addMissingExternalIds(k, workId, hint.externalIds); + if (addedIds.length > 0) after.externalIds = addedIds; + if (Object.keys(after).length === 0) return; + const now = nowIso(); + if (Object.keys(patch).length > 0) { + await k.updateTable("works").set({ ...patch, updated_at: now }).where("id", "=", workId).execute(); + } + await k + .insertInto("work_history") + .values({ id: newId(), work_id: workId, before: toJson(before), after: toJson(after), actor, created_at: now }) + .execute(); +} + +/** Fill or update a shared work's display fields from a hint, add missing external IDs, log to `work_history`. */ +export function updateWorkFromHint(db: Db, workId: string, hint: WorkHint, actor: string): Promise { + return db.k.transaction().execute((tx) => updateWorkFromHintIn(tx, workId, hint, actor)); +} + +function escapeLike(s: string): string { + return s.replace(/[!%_]/g, (c) => `!${c}`); +} + +/** Shared works plus the user's own private ones, case-insensitive substring match on title. */ +export async function searchWorks( + db: Db, + userId: string, + q: string, + opts: { mediaType?: MediaType; limit?: number } = {}, +): Promise { + let qb = db.k + .selectFrom("works") + .selectAll() + .where("merged_into_id", "is", null) + .where((eb) => eb.or([eb("owner_user_id", "is", null), eb("owner_user_id", "=", userId)])) + .where(sql`lower(title) like ${`%${escapeLike(q.toLowerCase())}%`} escape '!'`); + if (opts.mediaType) qb = qb.where("media_type", "=", opts.mediaType); + const rows = await qb.orderBy("title").limit(opts.limit ?? 50).execute(); + return rows.map(toWork); +} + +/** Follow the user's aliases and global merge tombstones to the work id they should see. */ +export async function resolveAliasIn(k: K, userId: string, workId: string): Promise { + const seen = new Set(); + let cur = workId; + while (!seen.has(cur)) { + seen.add(cur); + const alias = await k + .selectFrom("work_aliases") + .select("to_work_id") + .where("user_id", "=", userId) + .where("from_work_id", "=", cur) + .executeTakeFirst(); + if (alias) { + cur = alias.to_work_id; + continue; + } + const w = await k.selectFrom("works").select("merged_into_id").where("id", "=", cur).executeTakeFirst(); + if (!w?.merged_into_id) break; + cur = w.merged_into_id; + } + return cur; +} + +export function resolveAlias(db: Db, userId: string, workId: string): Promise { + return resolveAliasIn(db.k, userId, workId); +} + +/** + * Admin-only global merge of shared work `fromId` into `toId`. Leaves a `merged_into_id` tombstone and + * moves external IDs. Users without an entry on `toId` get their entry repointed; users who already + * have one keep their `from` entry and get a pending `merged` change event. + */ +export async function adminMergeWorks(db: Db, adminUserId: string, fromId: string, toId: string): Promise { + await db.k.transaction().execute(async (tx) => { + const admin = await tx.selectFrom("users").select("role").where("id", "=", adminUserId).executeTakeFirst(); + if (admin?.role !== "admin") throw new AuthError("forbidden"); + if (fromId === toId) throw new ValidationError("toId", "cannot merge a work into itself"); + const from = await tx.selectFrom("works").selectAll().where("id", "=", fromId).executeTakeFirst(); + const to = await tx.selectFrom("works").selectAll().where("id", "=", toId).executeTakeFirst(); + if (!from) throw new NotFoundError("work"); + if (!to) throw new NotFoundError("work"); + if (from.merged_into_id) throw new ValidationError("fromId", "already merged"); + if (to.merged_into_id) throw new ValidationError("toId", "already merged"); + const now = nowIso(); + + await tx.updateTable("works").set({ merged_into_id: toId, updated_at: now }).where("id", "=", fromId).execute(); + const moved = await tx.selectFrom("external_ids").select(["namespace", "value"]).where("work_id", "=", fromId).execute(); + await tx.updateTable("external_ids").set({ work_id: toId }).where("work_id", "=", fromId).execute(); + + const entries = await tx.selectFrom("entries").select(["id", "user_id"]).where("work_id", "=", fromId).execute(); + for (const e of entries) { + const existing = await tx + .selectFrom("entries") + .select("id") + .where("user_id", "=", e.user_id) + .where("work_id", "=", toId) + .executeTakeFirst(); + if (existing) { + await tx + .insertInto("change_events") + .values({ + id: newId(), + user_id: e.user_id, + connection_id: null, + run_id: null, + record_id: null, + type: "merged", + status: "pending", + target_entity: "entry", + target_id: e.id, + payload: toJson({ fromWorkId: fromId, toWorkId: toId }), + created_at: now, + resolved_at: null, + }) + .execute(); + } else { + await tx.updateTable("entries").set({ work_id: toId, updated_at: now }).where("id", "=", e.id).execute(); + } + } + + await tx + .insertInto("work_history") + .values({ id: newId(), work_id: fromId, before: toJson({ merged_into_id: null }), after: toJson({ merged_into_id: toId }), actor: adminUserId, created_at: now }) + .execute(); + await tx + .insertInto("work_history") + .values({ id: newId(), work_id: toId, before: "{}", after: toJson({ mergedFrom: fromId, externalIds: moved }), actor: adminUserId, created_at: now }) + .execute(); + }); +} diff --git a/packages/core/test/matching.test.ts b/packages/core/test/matching.test.ts new file mode 100644 index 0000000..f460c82 --- /dev/null +++ b/packages/core/test/matching.test.ts @@ -0,0 +1,102 @@ +import { expect, test } from "bun:test"; +import type { Db } from "@sttracker/db/db"; +import { newId, nowIso } from "@sttracker/db/ids"; +import { forEachDialect } from "@sttracker/db/testing"; +import { fuzzyScore, matchWork } from "../src/matching.ts"; +import { createWork, getWork } from "../src/works.ts"; + +async function addUser(db: Db): Promise { + const id = newId(); + const now = nowIso(); + await db.k + .insertInto("users") + .values({ id, username: `u${id}`, display_name: "U", password_hash: "x", role: "member", preferences: "{}", created_at: now, updated_at: now }) + .execute(); + return id; +} + +test("fuzzyScore", () => { + expect(fuzzyScore({ title: "The Expanse: Leviathan Wakes" }, { title: "Leviathan Wakes" })).toBeGreaterThan(0.6); + expect(fuzzyScore({ title: "Dune" }, { title: "Dune Messiah" })).toBeLessThan(0.6); + expect(fuzzyScore({ title: "Pokémon" }, { title: "Pokemon" })).toBe(1); + expect( + fuzzyScore({ title: "Pokémon", year: 1996, creators: [{ name: "Satoshi" }] }, { title: "pokemon!", year: 1996, creators: [{ name: "satoshi" }] }), + ).toBe(1); + // year off by one counts half, creator mismatch counts zero + expect(fuzzyScore({ title: "A B", year: 2000, creators: [{ name: "x" }] }, { title: "A B", year: 2001, creators: [{ name: "y" }] })).toBeCloseTo(0.8, 5); +}); + +forEachDialect("matching", (getDb) => { + test("match by isbn13 links existing", async () => { + const db = getDb(); + const u = await addUser(db); + const id = await createWork(db, { hint: { title: "Dune", mediaType: "book", externalIds: { isbn13: "9780441013593" } } }, "t"); + const r = await matchWork(db, u, { title: "Dune (Ace)", mediaType: "book", year: 1990, externalIds: { isbn13: "9780441013593", asin: "B1" } }, "plug"); + expect(r).toEqual({ workId: id, created: false, draft: false }); + expect((await getWork(db, id))?.year).toBe(1990); + }); + + test("unseen external id creates shared work", async () => { + const db = getDb(); + const u = await addUser(db); + const r = await matchWork(db, u, { title: "Heat", mediaType: "movie", externalIds: { tmdb_movie: "949" } }, "p"); + expect(r.created).toBe(true); + expect(r.draft).toBe(false); + expect((await getWork(db, r.workId))?.owner_user_id).toBeNull(); + }); + + const chain = () => ({ + title: "Pilot", + mediaType: "episode" as const, + externalIds: {}, + position: { episode: 1, season: 1 }, + parent: { + title: "Season 1", + mediaType: "season" as const, + externalIds: {}, + position: { season: 1 }, + parent: { title: "Lost", mediaType: "show" as const, externalIds: { tmdb_show: "4607" } }, + }, + }); + + test("episode hint creates show→season→episode chain once", async () => { + const db = getDb(); + const u = await addUser(db); + const r1 = await matchWork(db, u, chain(), "p"); + const r2 = await matchWork(db, u, chain(), "p"); + expect(r1.created).toBe(true); + expect(r2).toEqual({ workId: r1.workId, created: false, draft: false }); + expect(await db.k.selectFrom("works").select("id").execute()).toHaveLength(3); + const ep = await getWork(db, r1.workId); + expect(ep?.position).toEqual({ season: 1, episode: 1 }); + expect(ep?.parent_id).not.toBeNull(); + const season = await getWork(db, ep!.parent_id!); + expect(season?.media_type).toBe("season"); + expect((await getWork(db, season!.parent_id!))?.media_type).toBe("show"); + }); + + test("position key order does not matter", async () => { + const db = getDb(); + const u = await addUser(db); + const a = await matchWork(db, u, chain(), "p"); + const h = chain(); + h.position = { season: 1, episode: 1 }; + expect((await matchWork(db, u, h, "p")).workId).toBe(a.workId); + }); + + test("hint without ids creates private draft with suggestions", async () => { + const db = getDb(); + const u = await addUser(db); + const shared = await createWork(db, { hint: { title: "Leviathan Wakes", mediaType: "book", externalIds: { isbn13: "1" } } }, "t"); + await createWork(db, { hint: { title: "Leviathan Wakes", mediaType: "game", externalIds: { igdb: "1" } } }, "t"); + await createWork(db, { hint: { title: "Unrelated", mediaType: "book", externalIds: { isbn13: "2" } } }, "t"); + const r = await matchWork(db, u, { title: "The Expanse: Leviathan Wakes", mediaType: "book", externalIds: {} }, "p"); + expect(r.created).toBe(true); + expect(r.draft).toBe(true); + expect((await getWork(db, r.workId))?.owner_user_id).toBe(u); + const sug = await db.k.selectFrom("match_suggestions").selectAll().execute(); + expect(sug).toHaveLength(1); + expect(sug[0]).toMatchObject({ user_id: u, work_id: r.workId, candidate_work_id: shared, status: "pending" }); + expect(sug[0]!.score).toBeGreaterThan(0.6); + }); +}); diff --git a/packages/core/test/works.test.ts b/packages/core/test/works.test.ts new file mode 100644 index 0000000..1a7a4aa --- /dev/null +++ b/packages/core/test/works.test.ts @@ -0,0 +1,112 @@ +import { expect, test } from "bun:test"; +import type { Db } from "@sttracker/db/db"; +import { newId, nowIso } from "@sttracker/db/ids"; +import { fromJson } from "@sttracker/db/json"; +import { forEachDialect } from "@sttracker/db/testing"; +import { AuthError } from "../src/errors.ts"; +import { + adminMergeWorks, + createWork, + findWorkByExternalIds, + getWork, + resolveAlias, + searchWorks, + updateWorkFromHint, +} from "../src/works.ts"; + +async function addUser(db: Db, role: "admin" | "member" = "member"): Promise { + const id = newId(); + const now = nowIso(); + await db.k + .insertInto("users") + .values({ id, username: `u${id}`, display_name: "U", password_hash: "x", role, preferences: "{}", created_at: now, updated_at: now }) + .execute(); + return id; +} + +async function addEntry(db: Db, userId: string, workId: string): Promise { + const id = newId(); + const now = nowIso(); + await db.k + .insertInto("entries") + .values({ + id, user_id: userId, work_id: workId, status: "none", rating: null, review: null, private_notes: null, + favorite: 0, tags: "[]", custom_fields: "{}", visibility: "private", overrides: "{}", + media_type_override: null, field_sources: "{}", created_at: now, updated_at: now, deleted_at: null, + }) + .execute(); + return id; +} + +forEachDialect("works", (getDb) => { + test("createWork stores external ids; findWorkByExternalIds finds any namespace", async () => { + const db = getDb(); + const id = await createWork(db, { hint: { title: "Dune", mediaType: "book", year: 1965, externalIds: { isbn13: "9780441013593", hardcover_book: "7" } } }, "test"); + expect(await findWorkByExternalIds(db, { nope: "1", hardcover_book: "7" })).toBe(id); + expect(await findWorkByExternalIds(db, { isbn13: "0" })).toBeNull(); + expect((await getWork(db, id))?.year).toBe(1965); + }); + + test("updateWorkFromHint updates shared work, logs history, never touches private", async () => { + const db = getDb(); + const u = await addUser(db); + const shared = await createWork(db, { hint: { title: "Dune", mediaType: "book", externalIds: { a: "1" } } }, "t"); + await updateWorkFromHint(db, shared, { title: "Dune", mediaType: "book", year: 1965, description: "Sand", externalIds: { a: "1", b: "2" } }, "plug"); + const w = await getWork(db, shared); + expect(w?.year).toBe(1965); + expect(w?.description).toBe("Sand"); + expect(await findWorkByExternalIds(db, { b: "2" })).toBe(shared); + const hist = await db.k.selectFrom("work_history").selectAll().where("work_id", "=", shared).where("actor", "=", "plug").execute(); + expect(hist).toHaveLength(1); + expect(fromJson>(hist[0]!.before, {}).year).toBeNull(); + + const priv = await createWork(db, { hint: { title: "Mine", mediaType: "book", externalIds: {} }, ownerUserId: u }, "t"); + await updateWorkFromHint(db, priv, { title: "Changed", mediaType: "book", externalIds: {} }, "plug"); + expect((await getWork(db, priv))?.title).toBe("Mine"); + }); + + test("private works invisible to other users in search", async () => { + const db = getDb(); + const u1 = await addUser(db); + const u2 = await addUser(db); + await createWork(db, { hint: { title: "Secret 100%", mediaType: "book", externalIds: {} }, ownerUserId: u1 }, "t"); + await createWork(db, { hint: { title: "Public Thing", mediaType: "book", externalIds: { x: "1" } } }, "t"); + expect((await searchWorks(db, u1, "secret 100%")).map((w) => w.title)).toEqual(["Secret 100%"]); + expect(await searchWorks(db, u2, "secret")).toEqual([]); + expect((await searchWorks(db, u2, "PUBLIC")).map((w) => w.title)).toEqual(["Public Thing"]); + expect(await searchWorks(db, u2, "public", { mediaType: "game" })).toEqual([]); + }); + + test("adminMergeWorks requires admin", async () => { + const db = getDb(); + const member = await addUser(db); + const a = await createWork(db, { hint: { title: "A", mediaType: "book", externalIds: { x: "1" } } }, "t"); + const b = await createWork(db, { hint: { title: "B", mediaType: "book", externalIds: { x: "2" } } }, "t"); + await expect(adminMergeWorks(db, member, a, b)).rejects.toBeInstanceOf(AuthError); + }); + + test("adminMergeWorks tombstones, moves ids, repoints entries, emits merged event on conflict", async () => { + const db = getDb(); + const admin = await addUser(db, "admin"); + const u1 = await addUser(db); + const u2 = await addUser(db); + const from = await createWork(db, { hint: { title: "Old", mediaType: "book", externalIds: { x: "1" } } }, "t"); + const to = await createWork(db, { hint: { title: "New", mediaType: "book", externalIds: { x: "2" } } }, "t"); + const e1 = await addEntry(db, u1, from); + const e2from = await addEntry(db, u2, from); + await addEntry(db, u2, to); + + await adminMergeWorks(db, admin, from, to); + + expect((await getWork(db, from))?.id).toBe(to); + expect(await findWorkByExternalIds(db, { x: "1" })).toBe(to); + expect(await resolveAlias(db, u1, from)).toBe(to); + expect((await db.k.selectFrom("entries").select("work_id").where("id", "=", e1).executeTakeFirstOrThrow()).work_id).toBe(to); + expect((await db.k.selectFrom("entries").select("work_id").where("id", "=", e2from).executeTakeFirstOrThrow()).work_id).toBe(from); + const ev = await db.k.selectFrom("change_events").selectAll().execute(); + expect(ev).toHaveLength(1); + expect(ev[0]).toMatchObject({ user_id: u2, type: "merged", status: "pending", target_entity: "entry", target_id: e2from }); + expect(fromJson(ev[0]!.payload, {})).toEqual({ fromWorkId: from, toWorkId: to }); + expect(await db.k.selectFrom("work_history").select("id").where("work_id", "=", from).where("actor", "=", admin).execute()).toHaveLength(1); + }); +}); From 437ca143e500f87b85467cc3e11d81323d2e5196 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 19:29:48 +0100 Subject: [PATCH 2/3] fix(core): race-safe matching, visibility-checked ID matches, child-preserving merges - migration 0002: unique index on works (parent_id, position) - matchWork retries the whole transaction on a unique violation (3 attempts) - addMissingExternalIds uses ON CONFLICT DO NOTHING - ID matches only return shared works or the user's own; private works cannot carry external ids - adminMergeWorks requires shared works and repoints non-colliding children - draft is computed for the aliased work id Co-Authored-By: Claude Opus 5.5 --- docs/DECISIONS.md | 23 +++++ docs/TODO.md | 2 + packages/core/src/matching.ts | 36 ++++++-- packages/core/src/works.ts | 54 ++++++++++-- packages/core/test/matching.test.ts | 46 ++++++++++ packages/core/test/works.test.ts | 86 ++++++++++++++++++- .../migrations/0002_works_position_unique.ts | 16 ++++ packages/db/src/migrations/index.ts | 2 + packages/db/test/schema.test.ts | 23 ++++- 9 files changed, 270 insertions(+), 18 deletions(-) create mode 100644 packages/db/src/migrations/0002_works_position_unique.ts diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 2a7b147..b1787f4 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -81,3 +81,26 @@ instead of 26. Where `AGENTS.md` or the spec say "ULID", read "UUIDv7". **Consequences:** Rule changes happen in one file, and the pointer files must stay thin. Plans are working artefacts: kept after merge, they go stale and contradict the code, so the spec is the record. + +## 2026-10-30: Catalogue matching is race-safe through unique keys plus whole-transaction retry +**Status:** accepted +**Context:** Two syncs matching the same new work at once both miss the lookup and both insert. On +Postgres the loser hit the `external_ids (namespace, value)` unique key and crashed the sync; seasons and +episodes had no key at all and were duplicated. +**Decision:** +- Migration `0002_works_position_unique` adds a unique index on `works (parent_id, position)`. NULLs are + distinct on both dialects, so top-level works and position-less children are unaffected. +- `matchWork` re-runs the whole resolve in a fresh transaction on a unique violation (SQLite + "UNIQUE constraint failed", Postgres SQLSTATE 23505), up to 3 attempts. Postgres aborts the transaction + on the error, so a statement-level retry is not possible. +- Adding missing external IDs to an existing work uses `ON CONFLICT DO NOTHING`. +- An ID match only counts if the work is shared or owned by the matching user, and private works can + never carry external IDs (`createWork` refuses; global merges accept only shared works). +- A global merge repoints children of `from` to `to` unless `to` already has a child at that position. + +**Alternatives:** +- Advisory or table locks: dialect-specific, and SQLite already serialises writes. +- Upserts throughout `createWork`: would hide genuine conflicts and need per-dialect `RETURNING` handling. + +**Consequences:** Parents resolved without IDs or position (title/year reuse) still have no unique key +and can be duplicated under concurrency; tracked in `docs/TODO.md`. diff --git a/docs/TODO.md b/docs/TODO.md index d737840..8d583a3 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -23,4 +23,6 @@ Small, specific, independently actionable. Remove only when actually done. file, and compare steps and token use. ## Known issues +- [ ] Matching: a shared parent resolved without external IDs or position (title/year reuse in + `packages/core/src/matching.ts`) has no unique key, so concurrent matches can create it twice. - [ ] `bun run typecheck` fails with TS18003 until source files exist under `apps/`, `packages/` or `plugins/`. diff --git a/packages/core/src/matching.ts b/packages/core/src/matching.ts index ddc0563..3cf5a52 100644 --- a/packages/core/src/matching.ts +++ b/packages/core/src/matching.ts @@ -67,12 +67,12 @@ interface Resolved { } async function resolve(k: K, userId: string | null, hint: WorkHint, actor: string): Promise { - // 1. Any external ID. - const byId = await findWorkByExternalIdsIn(k, hint.externalIds); + // 1. Any external ID on a work this user may see (parents resolve with userId null: shared only). + const byId = await findWorkByExternalIdsIn(k, hint.externalIds, userId); if (byId) { await updateWorkFromHintIn(k, byId, hint, actor); - const owner = await k.selectFrom("works").select("owner_user_id").where("id", "=", byId).executeTakeFirstOrThrow(); const workId = userId ? await resolveAliasIn(k, userId, byId) : byId; + const owner = await k.selectFrom("works").select("owner_user_id").where("id", "=", workId).executeTakeFirstOrThrow(); return { workId, created: false, draft: owner.owner_user_id !== null }; } @@ -160,7 +160,31 @@ async function suggest(k: K, userId: string, draftId: string, hint: WorkHint): P } } -/** Resolve a hint to a work per spec §8.4. Never auto-links on fuzzy matches; those become suggestions. */ -export function matchWork(db: Db, userId: string, hint: WorkHint, actor: string): Promise { - return db.k.transaction().execute((tx) => resolve(tx, userId, hint, actor)); +const MAX_ATTEMPTS = 3; + +/** SQLite "UNIQUE constraint failed", or Postgres SQLSTATE 23505, anywhere in the cause chain. */ +export function isUniqueViolation(err: unknown): boolean { + for (let e: unknown = err, depth = 0; e && typeof e === "object" && depth < 5; e = (e as { cause?: unknown }).cause, depth++) { + const { code, errno, message } = e as { code?: unknown; errno?: unknown; message?: unknown }; + if (code === "23505" || errno === "23505") return true; + if (typeof message === "string" && (message.includes("UNIQUE constraint failed") || message.includes("duplicate key"))) { + return true; + } + } + return false; +} + +/** + * Resolve a hint to a work per spec §8.4. Never auto-links on fuzzy matches; those become suggestions. + * A concurrent match may create the same work first (unique external id or parent+position); the losing + * transaction is discarded and the whole resolve re-run, which then finds the winner's rows. + */ +export async function matchWork(db: Db, userId: string, hint: WorkHint, actor: string): Promise { + for (let attempt = 1; ; attempt++) { + try { + return await db.k.transaction().execute((tx) => resolve(tx, userId, hint, actor)); + } catch (err) { + if (attempt >= MAX_ATTEMPTS || !isUniqueViolation(err)) throw err; + } + } } diff --git a/packages/core/src/works.ts b/packages/core/src/works.ts index 2e9ccca..b5d68ae 100644 --- a/packages/core/src/works.ts +++ b/packages/core/src/works.ts @@ -45,6 +45,9 @@ export async function createWorkIn( ): Promise { const { hint } = input; if (hint.title.trim() === "") throw new ValidationError("title", "required"); + if (input.ownerUserId && Object.keys(hint.externalIds).length > 0) { + throw new ValidationError("externalIds", "private works cannot carry external ids"); + } const id = newId(); const now = nowIso(); await k @@ -111,7 +114,15 @@ export async function finalWorkId(k: K, id: string): Promise { return (await getWorkIn(k, id))?.id ?? id; } -export async function findWorkByExternalIdsIn(k: K, ids: Record): Promise { +/** + * First work matching any (namespace, value) pair, following merges. Only works visible to `userId` + * count (shared, or private to that user); with no user, only shared works. Others are treated as absent. + */ +export async function findWorkByExternalIdsIn( + k: K, + ids: Record, + userId: string | null = null, +): Promise { for (const [namespace, value] of Object.entries(ids)) { const row = await k .selectFrom("external_ids") @@ -119,14 +130,16 @@ export async function findWorkByExternalIdsIn(k: K, ids: Record) .where("namespace", "=", namespace) .where("value", "=", value) .executeTakeFirst(); - if (row) return finalWorkId(k, row.work_id); + if (!row) continue; + const work = await getWorkIn(k, row.work_id); + if (work && (work.owner_user_id === null || work.owner_user_id === userId)) return work.id; } return null; } -/** First work matching any (namespace, value) pair, following merges. */ -export function findWorkByExternalIds(db: Db, ids: Record): Promise { - return findWorkByExternalIdsIn(db.k, ids); +/** First work visible to `userId` (shared works only when omitted) matching any (namespace, value) pair. */ +export function findWorkByExternalIds(db: Db, ids: Record, userId: string | null = null): Promise { + return findWorkByExternalIdsIn(db.k, ids, userId); } async function addMissingExternalIds(k: K, workId: string, ids: Record): Promise { @@ -139,8 +152,13 @@ async function addMissingExternalIds(k: K, workId: string, ids: Record oc.columns(["namespace", "value"]).doNothing()) + .executeTakeFirst(); + if (Number(res.numInsertedOrUpdatedRows ?? 0n) > 0) added.push(`${namespace}:${value}`); } return added; } @@ -231,7 +249,8 @@ export function resolveAlias(db: Db, userId: string, workId: string): Promise { @@ -245,12 +264,29 @@ export async function adminMergeWorks(db: Db, adminUserId: string, fromId: strin if (!to) throw new NotFoundError("work"); if (from.merged_into_id) throw new ValidationError("fromId", "already merged"); if (to.merged_into_id) throw new ValidationError("toId", "already merged"); + if (from.owner_user_id !== null) throw new ValidationError("fromId", "only shared works can be merged"); + if (to.owner_user_id !== null) throw new ValidationError("toId", "only shared works can be merged"); const now = nowIso(); await tx.updateTable("works").set({ merged_into_id: toId, updated_at: now }).where("id", "=", fromId).execute(); const moved = await tx.selectFrom("external_ids").select(["namespace", "value"]).where("work_id", "=", fromId).execute(); await tx.updateTable("external_ids").set({ work_id: toId }).where("work_id", "=", fromId).execute(); + // Repoint children so matching through `toId` finds them. A child whose position `toId` already + // has stays under `fromId` (unique parent_id+position); it remains reachable via the tombstone. + const toPositions = new Set( + (await tx.selectFrom("works").select("position").where("parent_id", "=", toId).where("position", "is not", null).execute()).map( + (r) => r.position, + ), + ); + const children = await tx.selectFrom("works").select(["id", "position"]).where("parent_id", "=", fromId).execute(); + const movedChildren: string[] = []; + for (const c of children) { + if (c.position !== null && toPositions.has(c.position)) continue; + await tx.updateTable("works").set({ parent_id: toId, updated_at: now }).where("id", "=", c.id).execute(); + movedChildren.push(c.id); + } + const entries = await tx.selectFrom("entries").select(["id", "user_id"]).where("work_id", "=", fromId).execute(); for (const e of entries) { const existing = await tx @@ -288,7 +324,7 @@ export async function adminMergeWorks(db: Db, adminUserId: string, fromId: strin .execute(); await tx .insertInto("work_history") - .values({ id: newId(), work_id: toId, before: "{}", after: toJson({ mergedFrom: fromId, externalIds: moved }), actor: adminUserId, created_at: now }) + .values({ id: newId(), work_id: toId, before: "{}", after: toJson({ mergedFrom: fromId, externalIds: moved, children: movedChildren }), actor: adminUserId, created_at: now }) .execute(); }); } diff --git a/packages/core/test/matching.test.ts b/packages/core/test/matching.test.ts index f460c82..b47c09e 100644 --- a/packages/core/test/matching.test.ts +++ b/packages/core/test/matching.test.ts @@ -99,4 +99,50 @@ forEachDialect("matching", (getDb) => { expect(sug[0]).toMatchObject({ user_id: u, work_id: r.workId, candidate_work_id: shared, status: "pending" }); expect(sug[0]!.score).toBeGreaterThan(0.6); }); + + test("concurrent matches of the same new external id create one work", async () => { + const db = getDb(); + const u = await addUser(db); + const hint = () => ({ title: "Heat", mediaType: "movie" as const, externalIds: { tmdb_movie: "949" } }); + const rs = await Promise.all([matchWork(db, u, hint(), "p"), matchWork(db, u, hint(), "p"), matchWork(db, u, hint(), "p")]); + expect(new Set(rs.map((r) => r.workId)).size).toBe(1); + expect(await db.k.selectFrom("works").select("id").execute()).toHaveLength(1); + expect(await db.k.selectFrom("external_ids").select("id").execute()).toHaveLength(1); + }); + + test("concurrent matches of the same new episode chain create it once", async () => { + const db = getDb(); + const u = await addUser(db); + const rs = await Promise.all([matchWork(db, u, chain(), "p"), matchWork(db, u, chain(), "p"), matchWork(db, u, chain(), "p")]); + expect(new Set(rs.map((r) => r.workId)).size).toBe(1); + expect(await db.k.selectFrom("works").select("id").execute()).toHaveLength(3); + }); + + test("concurrent matches of new episodes under an existing show create one season", async () => { + const db = getDb(); + const u = await addUser(db); + await createWork(db, { hint: { title: "Lost", mediaType: "show", externalIds: { tmdb_show: "4607" } } }, "t"); + const rs = await Promise.all([matchWork(db, u, chain(), "p"), matchWork(db, u, chain(), "p")]); + expect(rs[0]!.workId).toBe(rs[1]!.workId); + expect(await db.k.selectFrom("works").select("id").execute()).toHaveLength(3); + }); + + test("concurrent updates adding the same external id do not fail", async () => { + const db = getDb(); + const u = await addUser(db); + const id = await createWork(db, { hint: { title: "Dune", mediaType: "book", externalIds: { isbn13: "1" } } }, "t"); + const hint = () => ({ title: "Dune", mediaType: "book" as const, externalIds: { isbn13: "1", asin: "B1" } }); + const rs = await Promise.all([matchWork(db, u, hint(), "p"), matchWork(db, u, hint(), "p"), matchWork(db, u, hint(), "p")]); + expect(rs.map((r) => r.workId)).toEqual([id, id, id]); + expect(await db.k.selectFrom("external_ids").select("id").where("namespace", "=", "asin").execute()).toHaveLength(1); + }); + + test("draft reflects the aliased work, not the matched one", async () => { + const db = getDb(); + const u = await addUser(db); + const shared = await createWork(db, { hint: { title: "Dune", mediaType: "book", externalIds: { isbn13: "1" } } }, "t"); + const priv = await createWork(db, { hint: { title: "My Dune", mediaType: "book", externalIds: {} }, ownerUserId: u }, "t"); + await db.k.insertInto("work_aliases").values({ id: newId(), user_id: u, from_work_id: shared, to_work_id: priv }).execute(); + expect(await matchWork(db, u, { title: "Dune", mediaType: "book", externalIds: { isbn13: "1" } }, "p")).toEqual({ workId: priv, created: false, draft: true }); + }); }); diff --git a/packages/core/test/works.test.ts b/packages/core/test/works.test.ts index 1a7a4aa..6ae7ed4 100644 --- a/packages/core/test/works.test.ts +++ b/packages/core/test/works.test.ts @@ -3,7 +3,8 @@ import type { Db } from "@sttracker/db/db"; import { newId, nowIso } from "@sttracker/db/ids"; import { fromJson } from "@sttracker/db/json"; import { forEachDialect } from "@sttracker/db/testing"; -import { AuthError } from "../src/errors.ts"; +import { AuthError, ValidationError } from "../src/errors.ts"; +import { matchWork } from "../src/matching.ts"; import { adminMergeWorks, createWork, @@ -109,4 +110,87 @@ forEachDialect("works", (getDb) => { expect(fromJson(ev[0]!.payload, {})).toEqual({ fromWorkId: from, toWorkId: to }); expect(await db.k.selectFrom("work_history").select("id").where("work_id", "=", from).where("actor", "=", admin).execute()).toHaveLength(1); }); + + test("createWork refuses external ids on a private work", async () => { + const db = getDb(); + const u = await addUser(db); + const p = createWork(db, { hint: { title: "Mine", mediaType: "book", externalIds: { isbn13: "1" } }, ownerUserId: u }, "t"); + await expect(p).rejects.toBeInstanceOf(ValidationError); + await expect(p).rejects.toMatchObject({ field: "externalIds", reason: "private works cannot carry external ids" }); + expect(await db.k.selectFrom("works").select("id").execute()).toHaveLength(0); + }); + + test("findWorkByExternalIds only returns works visible to the user", async () => { + const db = getDb(); + const a = await addUser(db); + const b = await addUser(db); + const priv = await createWork(db, { hint: { title: "Mine", mediaType: "book", externalIds: {} }, ownerUserId: a }, "t"); + // Legacy data: a private work carrying an external id (no longer creatable through createWork). + await db.k.insertInto("external_ids").values({ id: newId(), work_id: priv, namespace: "isbn13", value: "42" }).execute(); + expect(await findWorkByExternalIds(db, { isbn13: "42" }, a)).toBe(priv); + expect(await findWorkByExternalIds(db, { isbn13: "42" }, b)).toBeNull(); + expect(await findWorkByExternalIds(db, { isbn13: "42" })).toBeNull(); + const shared = await createWork(db, { hint: { title: "Theirs", mediaType: "book", externalIds: { asin: "X" } } }, "t"); + const hint = { title: "Mine", mediaType: "book" as const, externalIds: { isbn13: "42", asin: "X" } }; + expect(await matchWork(db, b, hint, "p")).toEqual({ workId: shared, created: false, draft: false }); + expect(await matchWork(db, a, hint, "p")).toEqual({ workId: priv, created: false, draft: true }); + }); + + test("getWork returns null on a merged_into cycle", async () => { + const db = getDb(); + const a = await createWork(db, { hint: { title: "A", mediaType: "book", externalIds: { x: "1" } } }, "t"); + const b = await createWork(db, { hint: { title: "B", mediaType: "book", externalIds: { x: "2" } } }, "t"); + await db.k.updateTable("works").set({ merged_into_id: b }).where("id", "=", a).execute(); + await db.k.updateTable("works").set({ merged_into_id: a }).where("id", "=", b).execute(); + expect(await getWork(db, a)).toBeNull(); + }); + + test("search treats a literal _ as a character, not a wildcard", async () => { + const db = getDb(); + const u = await addUser(db); + await createWork(db, { hint: { title: "snake_case", mediaType: "book", externalIds: { x: "1" } } }, "t"); + await createWork(db, { hint: { title: "snakeXcase", mediaType: "book", externalIds: { x: "2" } } }, "t"); + expect((await searchWorks(db, u, "e_c")).map((w) => w.title)).toEqual(["snake_case"]); + }); + + test("adminMergeWorks refuses private works", async () => { + const db = getDb(); + const admin = await addUser(db, "admin"); + const shared = await createWork(db, { hint: { title: "A", mediaType: "book", externalIds: { x: "1" } } }, "t"); + const priv = await createWork(db, { hint: { title: "B", mediaType: "book", externalIds: {} }, ownerUserId: admin }, "t"); + await expect(adminMergeWorks(db, admin, priv, shared)).rejects.toBeInstanceOf(ValidationError); + await expect(adminMergeWorks(db, admin, shared, priv)).rejects.toBeInstanceOf(ValidationError); + expect((await getWork(db, priv))?.id).toBe(priv); + }); + + test("adminMergeWorks moves children that do not collide, so matching does not recreate them", async () => { + const db = getDb(); + const admin = await addUser(db, "admin"); + const u = await addUser(db); + const show = (id: string) => ({ title: "Lost", mediaType: "show" as const, externalIds: { tmdb_show: id } }); + const ep = (showId: string, season: number) => ({ + title: "Pilot", + mediaType: "episode" as const, + externalIds: {}, + position: { season, episode: 1 }, + parent: { title: `Season ${season}`, mediaType: "season" as const, externalIds: {}, position: { season }, parent: show(showId) }, + }); + const a1 = await matchWork(db, u, ep("A", 1), "p"); + const a2 = await matchWork(db, u, ep("A", 2), "p"); + const b2 = await matchWork(db, u, ep("B", 2), "p"); + const showA = (await findWorkByExternalIds(db, { tmdb_show: "A" }))!; + const showB = (await findWorkByExternalIds(db, { tmdb_show: "B" }))!; + const seasonA1 = (await getWork(db, a1.workId))!.parent_id!; + const seasonA2 = (await getWork(db, a2.workId))!.parent_id!; + const before = (await db.k.selectFrom("works").select("id").execute()).length; + + await adminMergeWorks(db, admin, showA, showB); + + expect((await getWork(db, seasonA1))?.parent_id).toBe(showB); // moved: B had no season 1 + expect((await getWork(db, seasonA2))?.parent_id).toBe(showA); // left: B already had season 2 + const again = await matchWork(db, u, ep("A", 1), "p"); + expect(again).toEqual({ workId: a1.workId, created: false, draft: false }); + expect((await matchWork(db, u, ep("A", 2), "p")).workId).toBe(b2.workId); + expect((await db.k.selectFrom("works").select("id").execute()).length).toBe(before); + }); }); diff --git a/packages/db/src/migrations/0002_works_position_unique.ts b/packages/db/src/migrations/0002_works_position_unique.ts new file mode 100644 index 0000000..759af4f --- /dev/null +++ b/packages/db/src/migrations/0002_works_position_unique.ts @@ -0,0 +1,16 @@ +import type { Kysely } from "kysely"; +import type { Migration } from "kysely/migration"; + +/** + * One child per (parent, position), so concurrent matches cannot duplicate seasons or episodes. + * NULLs are distinct on both dialects: top-level works and children without a position are unaffected. + */ +export const worksPositionUnique: Migration = { + async up(db: Kysely) { + await db.schema.createIndex("works_parent_id_position_uq").unique().on("works").columns(["parent_id", "position"]).execute(); + }, + + async down(db: Kysely) { + await db.schema.dropIndex("works_parent_id_position_uq").execute(); + }, +}; diff --git a/packages/db/src/migrations/index.ts b/packages/db/src/migrations/index.ts index c8c4ed7..4dc1ca0 100644 --- a/packages/db/src/migrations/index.ts +++ b/packages/db/src/migrations/index.ts @@ -1,7 +1,9 @@ import type { Migration } from "kysely/migration"; import { initial } from "./0001_initial.ts"; +import { worksPositionUnique } from "./0002_works_position_unique.ts"; /** All migrations, keyed by sortable name. Imported statically so bundles need no filesystem. */ export const migrations: Record = { "0001_initial": initial, + "0002_works_position_unique": worksPositionUnique, }; diff --git a/packages/db/test/schema.test.ts b/packages/db/test/schema.test.ts index 6bef1c2..e19cbd5 100644 --- a/packages/db/test/schema.test.ts +++ b/packages/db/test/schema.test.ts @@ -1,6 +1,6 @@ import { expect, test } from "bun:test"; import { sql, type Kysely } from "kysely"; -import { Migrator } from "kysely/migration"; +import { Migrator, NO_MIGRATIONS } from "kysely/migration"; import { newId, nowIso } from "../src/ids.ts"; import { toJson } from "../src/json.ts"; import { migrations } from "../src/migrations/index.ts"; @@ -79,10 +79,29 @@ forEachDialect("schema", (getDb) => { await expect(k.insertInto("entries").values({ id: newId(), ...entry }).execute()).rejects.toThrow(); }); + test("works (parent_id, position) is unique; null positions and null parents are not", async () => { + const k = getDb().k; + const now = nowIso(); + const work = (id: string, parent: string | null, position: string | null) => ({ + id, media_type: "show", title: "T", original_title: "", year: null, release_date: null, creators: "[]", + description: "", cover_url: "", cover_attachment_id: null, metadata: "{}", parent_id: parent, position, + owner_user_id: null, merged_into_id: null, created_at: now, updated_at: now, + }); + const show = newId(); + await k.insertInto("works").values(work(show, null, null)).execute(); + await k.insertInto("works").values(work(newId(), null, null)).execute(); + await k.insertInto("works").values(work(newId(), null, '{"season":1}')).execute(); + await k.insertInto("works").values(work(newId(), null, '{"season":1}')).execute(); + await k.insertInto("works").values(work(newId(), show, null)).execute(); + await k.insertInto("works").values(work(newId(), show, null)).execute(); + await k.insertInto("works").values(work(newId(), show, '{"season":1}')).execute(); + await expect(k.insertInto("works").values(work(newId(), show, '{"season":1}')).execute()).rejects.toThrow(); + }); + test("migrating down removes the tables", async () => { const k = getDb().k as Kysely; const migrator = new Migrator({ db: k, provider: { getMigrations: async () => migrations } }); - const { error } = await migrator.migrateDown(); + const { error } = await migrator.migrateTo(NO_MIGRATIONS); expect(error).toBeUndefined(); const probe = sql`select 1 from users`.execute(k); await expect(probe).rejects.toThrow(); From 50c115b4c7a564a04ea608dbd80787db21e8e07a Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 19:32:27 +0100 Subject: [PATCH 3/3] fix(core): follow merged tombstones in the position lookup A merged child still holds its (parent_id, position) slot under the unique index, so skipping tombstones made every retry collide. Follow the tombstone to the surviving work instead. Co-Authored-By: Claude Opus 5.5 --- docs/TODO.md | 1 + packages/core/src/matching.ts | 8 +++++--- packages/core/test/matching.test.ts | 17 ++++++++++++++++- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/docs/TODO.md b/docs/TODO.md index 8d583a3..9eb62bc 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -23,6 +23,7 @@ Small, specific, independently actionable. Remove only when actually done. file, and compare steps and token use. ## Known issues +- [ ] Admin merge does not merge colliding children recursively; colliding children stay under the tombstoned parent. - [ ] Matching: a shared parent resolved without external IDs or position (title/year reuse in `packages/core/src/matching.ts`) has no unique key, so concurrent matches can create it twice. - [ ] `bun run typecheck` fails with TS18003 until source files exist under `apps/`, `packages/` or `plugins/`. diff --git a/packages/core/src/matching.ts b/packages/core/src/matching.ts index 3cf5a52..ac78f7b 100644 --- a/packages/core/src/matching.ts +++ b/packages/core/src/matching.ts @@ -4,6 +4,7 @@ import type { Db } from "@sttracker/db/db"; import { newId, nowIso } from "@sttracker/db/ids"; import { createWorkIn, + finalWorkId, findWorkByExternalIdsIn, normalizePosition, resolveAliasIn, @@ -85,12 +86,13 @@ async function resolve(k: K, userId: string | null, hint: WorkHint, actor: strin .select("id") .where("parent_id", "=", parentId) .where("position", "=", position) - .where("merged_into_id", "is", null) .where("owner_user_id", "is", null) .executeTakeFirst(); if (child) { - await updateWorkFromHintIn(k, child.id, hint, actor); - return { workId: child.id, created: false, draft: false }; + // A merged tombstone still holds its (parent_id, position) slot: follow it to the surviving work. + const workId = await finalWorkId(k, child.id); + await updateWorkFromHintIn(k, workId, hint, actor); + return { workId, created: false, draft: false }; } return { workId: await createWorkIn(k, { hint, parentId }, actor), created: true, draft: false }; } diff --git a/packages/core/test/matching.test.ts b/packages/core/test/matching.test.ts index b47c09e..c80f4bb 100644 --- a/packages/core/test/matching.test.ts +++ b/packages/core/test/matching.test.ts @@ -3,7 +3,7 @@ import type { Db } from "@sttracker/db/db"; import { newId, nowIso } from "@sttracker/db/ids"; import { forEachDialect } from "@sttracker/db/testing"; import { fuzzyScore, matchWork } from "../src/matching.ts"; -import { createWork, getWork } from "../src/works.ts"; +import { adminMergeWorks, createWork, getWork } from "../src/works.ts"; async function addUser(db: Db): Promise { const id = newId(); @@ -145,4 +145,19 @@ forEachDialect("matching", (getDb) => { await db.k.insertInto("work_aliases").values({ id: newId(), user_id: u, from_work_id: shared, to_work_id: priv }).execute(); expect(await matchWork(db, u, { title: "Dune", mediaType: "book", externalIds: { isbn13: "1" } }, "p")).toEqual({ workId: priv, created: false, draft: true }); }); + + test("position lookup follows a merged tombstone instead of colliding with it", async () => { + const db = getDb(); + const u = await addUser(db); + await db.k.updateTable("users").set({ role: "admin" }).where("id", "=", u).execute(); + const e1 = await matchWork(db, u, chain(), "p"); + const h2 = chain(); + h2.title = "Episode 2"; + h2.position = { season: 1, episode: 2 }; + const e2 = await matchWork(db, u, h2, "p"); + await adminMergeWorks(db, u, e1.workId, e2.workId); + const before = (await db.k.selectFrom("works").select("id").execute()).length; + expect(await matchWork(db, u, chain(), "p")).toEqual({ workId: e2.workId, created: false, draft: false }); + expect(await db.k.selectFrom("works").select("id").execute()).toHaveLength(before); + }); });