From 588be4e8584bf00e316e9d27a31ce4e9b4d9a591 Mon Sep 17 00:00:00 2001 From: Patrick Erichsen Date: Thu, 23 Jul 2026 15:01:02 -0700 Subject: [PATCH] fix: repair both sides of skill lineage cycles (#3248) --- convex/maintenance.test.ts | 68 +++++++++--- convex/maintenance.ts | 211 +++++++++++++++++++++---------------- 2 files changed, 176 insertions(+), 103 deletions(-) diff --git a/convex/maintenance.test.ts b/convex/maintenance.test.ts index b8db6da5..50474a32 100644 --- a/convex/maintenance.test.ts +++ b/convex/maintenance.test.ts @@ -1574,7 +1574,7 @@ function makeSkillLineageCycleDb(options?: { includeMergeAudit?: boolean }) { const sourceSkill = { _id: "skills:source", slug: "archive-graincrawl", - canonicalSkillId: "skills:final", + canonicalSkillId: "skills:source", forkOf: { skillId: "skills:final", kind: "duplicate", @@ -1629,18 +1629,29 @@ describe("maintenance skill lineage cycle repair", () => { it("recognizes the exact malformed merge pair from its audit history", async () => { const fixture = makeSkillLineageCycleDb(); - const result = await inspectSkillLineageCycleInternalHandler( + const finalResult = await inspectSkillLineageCycleInternalHandler( fixture as never, fixture.finalSkill._id as never, ); + const sourceResult = await inspectSkillLineageCycleInternalHandler( + fixture as never, + fixture.sourceSkill._id as never, + ); - expect(result).toEqual({ + expect(finalResult).toEqual({ status: "repairable", skillId: "skills:final", slug: "graincrawl", sourceSkillId: "skills:source", sourceSlug: "archive-graincrawl", }); + expect(sourceResult).toEqual({ + status: "paired_source", + skillId: "skills:source", + slug: "archive-graincrawl", + finalSkillId: "skills:final", + finalSlug: "graincrawl", + }); }); it("leaves a self-reference untouched without matching merge history", async () => { @@ -1652,16 +1663,16 @@ describe("maintenance skill lineage cycle repair", () => { ); expect(result).toEqual({ - status: "ambiguous", + status: "uncertain", skillId: "skills:final", slug: "graincrawl", reason: "missing_matching_merge_audit", - sourceSkillId: "skills:source", - sourceSlug: "archive-graincrawl", + linkedSkillId: "skills:source", + linkedSlug: "archive-graincrawl", }); }); - it("clears only the final skill lineage and writes an audit record", async () => { + it("repairs both sides of the pair and writes their prior state to the audit", async () => { const fixture = makeSkillLineageCycleDb(); const result = await applySkillLineageCycleRepairInternalHandler(fixture as never, { @@ -1670,7 +1681,7 @@ describe("maintenance skill lineage cycle repair", () => { }); expect(result).toEqual({ repaired: true }); - expect(fixture.patch).toHaveBeenCalledTimes(1); + expect(fixture.patch).toHaveBeenCalledTimes(2); expect(fixture.patch).toHaveBeenCalledWith( "skills:final", expect.objectContaining({ @@ -1678,6 +1689,13 @@ describe("maintenance skill lineage cycle repair", () => { forkOf: undefined, }), ); + expect(fixture.patch).toHaveBeenCalledWith( + "skills:source", + expect.objectContaining({ + canonicalSkillId: "skills:final", + forkOf: fixture.sourceSkill.forkOf, + }), + ); expect(fixture.insert).toHaveBeenCalledWith( "auditLogs", expect.objectContaining({ @@ -1686,24 +1704,38 @@ describe("maintenance skill lineage cycle repair", () => { targetId: "skills:final", metadata: expect.objectContaining({ sourceSkillId: "skills:source", - previousCanonicalSkillId: "skills:source", - previousForkOf: fixture.finalSkill.forkOf, + previousFinalCanonicalSkillId: "skills:source", + previousFinalForkOf: fixture.finalSkill.forkOf, + previousSourceCanonicalSkillId: "skills:source", + previousSourceForkOf: fixture.sourceSkill.forkOf, }), }), ); }); it("defaults to preview and reports resumable progress", async () => { - const runQuery = vi.fn(async (endpoint: unknown) => { + const runQuery = vi.fn(async (endpoint: unknown, args?: { skillId?: string }) => { if (endpoint === internal.maintenance.getSkillLineageCycleRepairPageInternal) { return { - items: [{ skillId: "skills:final", slug: "graincrawl" }], + items: [ + { skillId: "skills:final", slug: "graincrawl" }, + { skillId: "skills:source", slug: "archive-graincrawl" }, + ], scanned: 200, cursor: "next-page", isDone: false, }; } if (endpoint === internal.maintenance.inspectSkillLineageCycleInternal) { + if (args?.skillId === "skills:source") { + return { + status: "paired_source", + skillId: "skills:source", + slug: "archive-graincrawl", + finalSkillId: "skills:final", + finalSlug: "graincrawl", + }; + } return { status: "repairable", skillId: "skills:final", @@ -1729,9 +1761,10 @@ describe("maintenance skill lineage cycle repair", () => { isDone: false, stats: { skillsScanned: 200, - selfReferencesFound: 1, + selfReferencesFound: 2, repairable: 1, - ambiguous: 0, + pairedSources: 1, + uncertain: 0, repaired: 0, changedBeforeApply: 0, }, @@ -1743,6 +1776,13 @@ describe("maintenance skill lineage cycle repair", () => { sourceSkillId: "skills:source", sourceSlug: "archive-graincrawl", }, + { + status: "paired_source", + skillId: "skills:source", + slug: "archive-graincrawl", + finalSkillId: "skills:final", + finalSlug: "graincrawl", + }, ], }); expect(runMutation).not.toHaveBeenCalled(); diff --git a/convex/maintenance.ts b/convex/maintenance.ts index 8cc4d179..afa9961b 100644 --- a/convex/maintenance.ts +++ b/convex/maintenance.ts @@ -200,25 +200,33 @@ type SkillLineageCycleInspection = sourceSlug: string; } | { - status: "ambiguous"; + status: "paired_source"; + skillId: Id<"skills">; + slug: string; + finalSkillId: Id<"skills">; + finalSlug: string; + } + | { + status: "uncertain"; skillId: Id<"skills">; slug: string; reason: | "missing_skill" | "no_self_reference" - | "multiple_linked_sources" - | "missing_source" - | "source_not_merged_into_skill" + | "unexpected_self_reference_shape" + | "missing_linked_skill" + | "pair_shape_mismatch" | "missing_matching_merge_audit"; - sourceSkillId?: Id<"skills">; - sourceSlug?: string; + linkedSkillId?: Id<"skills">; + linkedSlug?: string; }; type SkillLineageCycleRepairStats = { skillsScanned: number; selfReferencesFound: number; repairable: number; - ambiguous: number; + pairedSources: number; + uncertain: number; repaired: number; changedBeforeApply: number; }; @@ -244,7 +252,11 @@ export type SkillLineageCycleRepairResult = { slug: string; sourceSkillId?: Id<"skills">; sourceSlug?: string; - reason?: Extract["reason"]; + finalSkillId?: Id<"skills">; + finalSlug?: string; + linkedSkillId?: Id<"skills">; + linkedSlug?: string; + reason?: Extract["reason"]; }>; }; @@ -2756,7 +2768,7 @@ export async function inspectSkillLineageCycleInternalHandler( const skill = await ctx.db.get(skillId); if (!skill) { return { - status: "ambiguous", + status: "uncertain", skillId, slug: "", reason: "missing_skill", @@ -2767,119 +2779,121 @@ export async function inspectSkillLineageCycleInternalHandler( skill.canonicalSkillId === skill._id || skill.forkOf?.skillId === skill._id; if (!hasSelfReference) { return { - status: "ambiguous", + status: "uncertain", skillId, slug: skill.slug, reason: "no_self_reference", }; } - const linkedSourceIds = new Set( - [skill.canonicalSkillId, skill.forkOf?.skillId].filter((linkedId): linkedId is Id<"skills"> => - Boolean(linkedId && linkedId !== skill._id), - ), - ); - if (linkedSourceIds.size > 1) { + const isFinalShape = + skill.softDeletedAt === undefined && + skill.canonicalSkillId !== undefined && + skill.canonicalSkillId !== skill._id && + skill.forkOf?.skillId === skill._id && + skill.forkOf.kind === "duplicate"; + const isSourceShape = + skill.softDeletedAt !== undefined && + skill.moderationStatus === "hidden" && + skill.moderationReason === "owner.merged" && + skill.canonicalSkillId === skill._id && + skill.forkOf !== undefined && + skill.forkOf.skillId !== skill._id && + skill.forkOf.kind === "duplicate"; + if (!isFinalShape && !isSourceShape) { return { - status: "ambiguous", + status: "uncertain", skillId, slug: skill.slug, - reason: "multiple_linked_sources", + reason: "unexpected_self_reference_shape", }; } - let source: Doc<"skills"> | null = null; - const directSourceId = [...linkedSourceIds][0]; - if (directSourceId) { - source = await ctx.db.get(directSourceId); - } else { - const [canonicalRefs, forkRefs] = await Promise.all([ - ctx.db - .query("skills") - .withIndex("by_canonical", (q) => q.eq("canonicalSkillId", skill._id)) - .take(3), - ctx.db - .query("skills") - .withIndex("by_fork_of", (q) => q.eq("forkOf.skillId", skill._id)) - .take(3), - ]); - const reverseSources = new Map, Doc<"skills">>(); - for (const related of [...canonicalRefs, ...forkRefs]) { - if (related._id !== skill._id) reverseSources.set(related._id, related); - } - const exactSources = [...reverseSources.values()].filter( - (related) => - related.canonicalSkillId === skill._id && - related.forkOf?.skillId === skill._id && - related.forkOf.kind === "duplicate", - ); - if (exactSources.length === 1) source = exactSources[0]; - if (exactSources.length > 1) { - return { - status: "ambiguous", - skillId, - slug: skill.slug, - reason: "multiple_linked_sources", - }; - } - } - - if (!source) { + const linkedSkillId: Id<"skills"> | undefined = isFinalShape + ? skill.canonicalSkillId + : skill.forkOf?.skillId; + if (!linkedSkillId || linkedSkillId === skill._id) { return { - status: "ambiguous", + status: "uncertain", skillId, slug: skill.slug, - reason: "missing_source", - ...(directSourceId ? { sourceSkillId: directSourceId } : {}), + reason: "unexpected_self_reference_shape", + }; + } + const linkedSkill: Doc<"skills"> | null = await ctx.db.get(linkedSkillId); + if (linkedSkill === null) { + return { + status: "uncertain", + skillId, + slug: skill.slug, + reason: "missing_linked_skill", + linkedSkillId, }; } - const sourceMatchesMergeState = - source.canonicalSkillId === skill._id && - source.forkOf?.skillId === skill._id && - source.forkOf.kind === "duplicate" && - source.softDeletedAt !== undefined && - source.moderationStatus === "hidden" && - source.moderationReason === "owner.merged"; - if (!sourceMatchesMergeState) { + const finalSkill: Doc<"skills"> = isFinalShape ? skill : linkedSkill; + const sourceSkill: Doc<"skills"> = isSourceShape ? skill : linkedSkill; + const pairMatches = + finalSkill.softDeletedAt === undefined && + finalSkill.canonicalSkillId === sourceSkill._id && + finalSkill.forkOf?.skillId === finalSkill._id && + finalSkill.forkOf.kind === "duplicate" && + sourceSkill.softDeletedAt !== undefined && + sourceSkill.moderationStatus === "hidden" && + sourceSkill.moderationReason === "owner.merged" && + sourceSkill.canonicalSkillId === sourceSkill._id && + sourceSkill.forkOf?.skillId === finalSkill._id && + sourceSkill.forkOf.kind === "duplicate"; + if (!pairMatches) { return { - status: "ambiguous", + status: "uncertain", skillId, slug: skill.slug, - reason: "source_not_merged_into_skill", - sourceSkillId: source._id, - sourceSlug: source.slug, + reason: "pair_shape_mismatch", + linkedSkillId: linkedSkill._id, + linkedSlug: linkedSkill.slug, }; } const mergeAuditLogs = await ctx.db .query("auditLogs") .withIndex("by_target_action", (q) => - q.eq("targetType", "skill").eq("targetId", source._id).eq("action", "skill.merge"), + q.eq("targetType", "skill").eq("targetId", sourceSkill._id).eq("action", "skill.merge"), ) .order("desc") .take(10); const matchingAudit = mergeAuditLogs.some( (log) => - log.createdAt === source.forkOf?.at && parseSkillMergeTargetId(log.metadata) === skill._id, + log.createdAt === sourceSkill.forkOf?.at && + parseSkillMergeTargetId(log.metadata) === finalSkill._id, ); if (!matchingAudit) { return { - status: "ambiguous", + status: "uncertain", skillId, slug: skill.slug, reason: "missing_matching_merge_audit", - sourceSkillId: source._id, - sourceSlug: source.slug, + linkedSkillId: linkedSkill._id, + linkedSlug: linkedSkill.slug, + }; + } + + if (isSourceShape) { + return { + status: "paired_source", + skillId: sourceSkill._id, + slug: sourceSkill.slug, + finalSkillId: finalSkill._id, + finalSlug: finalSkill.slug, }; } return { status: "repairable", - skillId, - slug: skill.slug, - sourceSkillId: source._id, - sourceSlug: source.slug, + skillId: finalSkill._id, + slug: finalSkill.slug, + sourceSkillId: sourceSkill._id, + sourceSlug: sourceSkill.slug, }; } @@ -2904,8 +2918,11 @@ export async function applySkillLineageCycleRepairInternalHandler( }; } - const skill = await ctx.db.get(args.skillId); - if (!skill) { + const [finalSkill, sourceSkill] = await Promise.all([ + ctx.db.get(args.skillId), + ctx.db.get(args.sourceSkillId), + ]); + if (!finalSkill || !sourceSkill || !sourceSkill.forkOf) { return { repaired: false as const, reason: "changed_before_apply" as const, @@ -2913,22 +2930,32 @@ export async function applySkillLineageCycleRepairInternalHandler( } const now = Date.now(); - await ctx.db.patch(skill._id, { + await ctx.db.patch(finalSkill._id, { canonicalSkillId: undefined, forkOf: undefined, updatedAt: now, }); + await ctx.db.patch(sourceSkill._id, { + canonicalSkillId: finalSkill._id, + forkOf: { + ...sourceSkill.forkOf, + skillId: finalSkill._id, + }, + updatedAt: now, + }); await ctx.db.insert("auditLogs", { action: "skill.lineage_cycle.repair", targetType: "skill", - targetId: skill._id, + targetId: finalSkill._id, metadata: { repairVersion: "skill-lineage-cycle-2026-07-23", - slug: skill.slug, + slug: finalSkill.slug, sourceSkillId: inspection.sourceSkillId, sourceSlug: inspection.sourceSlug, - previousCanonicalSkillId: skill.canonicalSkillId, - previousForkOf: skill.forkOf, + previousFinalCanonicalSkillId: finalSkill.canonicalSkillId, + previousFinalForkOf: finalSkill.forkOf, + previousSourceCanonicalSkillId: sourceSkill.canonicalSkillId, + previousSourceForkOf: sourceSkill.forkOf, }, createdAt: now, }); @@ -2961,7 +2988,8 @@ export async function repairSkillLineageCyclesInternalHandler( skillsScanned: 0, selfReferencesFound: 0, repairable: 0, - ambiguous: 0, + pairedSources: 0, + uncertain: 0, repaired: 0, changedBeforeApply: 0, }; @@ -2985,8 +3013,13 @@ export async function repairSkillLineageCyclesInternalHandler( { skillId: item.skillId }, )) as SkillLineageCycleInspection; - if (inspection.status === "ambiguous") { - stats.ambiguous++; + if (inspection.status === "uncertain") { + stats.uncertain++; + if (samples.length < 200) samples.push(inspection); + continue; + } + if (inspection.status === "paired_source") { + stats.pairedSources++; if (samples.length < 200) samples.push(inspection); continue; }