From 1158c4f683fee7caa2b5762760bf21e9bda3859b Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 14 Jul 2026 13:28:02 -0400 Subject: [PATCH] PROD-2307: adopt existing target containers by referenceName (mapping self-heal) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defensive hardening, not the fix for the ticket's headline symptom. When the local mapping cache is absent (fresh machine, CI runner, wiped agility-files, or a create that succeeded server-side but was never mapped), container-pusher had no fallback: with no mapping row it always took the create path. Agility's saveContainer dedupes by referenceName so this rarely produced a visible duplicate container, but the missing mapping row is the real damage — downstream content/pages resolve container references through hasValidMappings, and a missing container mapping tips content into the create path, which is what feeds duplicate accumulation across syncs. This mirrors the model-pusher PROD-2211 map-on-adopt path: before creating, look for an existing target container matching by referenceName (unique per instance) and, if found, write the mapping row and skip instead of creating. Reference names are matched case-insensitively; the mapping write is skipped under --preflight; a same-name/different-model match logs a warning. What this does NOT do: it does not resolve the ticket's divergent-hash nested container doubling (orphans with names the source never had), which requires the source/target nested-container names to diverge — a path not reproducible on the current build. That is handled separately (orphan cleanup / PROD-2282), and the content-item duplication observed under a missing mapping cache is a distinct root cause tracked on its own. Verified: fresh-state sync now reports containers adopted (skipped) with mapping rows written instead of re-created; a second sync is stable. Full suite green (1732 tests); 4 new container-pusher unit tests cover adopt, case-insensitive match, still-creates-when-no-match, and preflight-no-write. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/lib/pushers/container-pusher.ts | 39 +++++++++ .../pushers/tests/container-pusher.test.ts | 79 +++++++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/src/lib/pushers/container-pusher.ts b/src/lib/pushers/container-pusher.ts index 41ed514c..4feb3e12 100644 --- a/src/lib/pushers/container-pusher.ts +++ b/src/lib/pushers/container-pusher.ts @@ -78,6 +78,45 @@ export async function pushContainers( // if no mapping found, we should be creating the container shouldCreate = existingMapping === null; + // PROD-2307: before creating, adopt an existing target container that matches by + // referenceName but has no mapping row (e.g. a fresh machine / wiped mapping cache, + // or a create that succeeded server-side but was never mapped). Container reference + // names are unique per instance, so a name match is a safe identity. This mirrors the + // model-pusher PROD-2211 map-on-adopt path: without it, a mapping-less run re-creates + // containers and drops the mapping row that downstream content/pages rely on. + if (shouldCreate) { + const targetByRef = + targetData?.find( + (tc: mgmtApi.Container) => + tc.referenceName?.toLowerCase() === sourceContainer.referenceName?.toLowerCase() + ) || null; + + if (targetByRef) { + if (targetModelID >= 1 && targetByRef.contentDefinitionID !== targetModelID) { + // Pathological: same reference name, different model. Adopt on the (unique) name, + // but surface the mismatch so it can be investigated. + logger.log( + "WARN", + `Adopting existing target container "${sourceContainer.referenceName}" by referenceName, ` + + `but its model (${targetByRef.contentDefinitionID}) differs from the mapped source model (${targetModelID}).` + ); + } + + if (!state.preflight) { + containerMapper.addMapping(sourceContainer, targetByRef); + } + logger.container.skipped(sourceContainer, "already exists on target; mapping row created", targetGuid[0]); + preflightReport.record({ + phase: "Containers", + action: "skip", + name: sourceContainer.referenceName, + detail: "adopted existing target container (mapping row created)", + }); + skipped++; + continue; // finally{} still runs (processedCount++); no create/update this run + } + } + if (!shouldCreate) { // get the target container, check if the source and targets need updates const targetContainer: mgmtApi.Container = diff --git a/src/lib/pushers/tests/container-pusher.test.ts b/src/lib/pushers/tests/container-pusher.test.ts index c615306e..ab22c5f3 100644 --- a/src/lib/pushers/tests/container-pusher.test.ts +++ b/src/lib/pushers/tests/container-pusher.test.ts @@ -154,6 +154,85 @@ describe("pushContainers — RichTextArea special case", () => { }); }); +// ─── pushContainers — PROD-2307 adopt-by-referenceName ──────────────────────── + +describe("pushContainers — adopt existing target container by referenceName (PROD-2307)", () => { + it("adopts an unmapped target container that matches by referenceName instead of creating", async () => { + const saveContainer = jest.fn(); + state.cachedApiClient = { containerMethods: { saveContainer } } as any; + + const { ContainerMapper } = await import("lib/mappers/container-mapper"); + const addMappingSpy = jest.spyOn(ContainerMapper.prototype, "addMapping"); + + const { pushContainers } = await import("../container-pusher"); + + // Source has no mapping row (fresh cache); target already has a same-named container. + const source = makeContainer({ referenceName: "AdoptMe", contentDefinitionID: 500 }); + const target = makeContainer({ referenceName: "AdoptMe", contentDefinitionID: 500 }); + + const result = await pushContainers([source], [target]); + + // Adopted, not created. + expect(saveContainer).not.toHaveBeenCalled(); + expect(addMappingSpy).toHaveBeenCalledTimes(1); + expect(result.skipped).toBe(1); + expect(result.successful).toBe(0); + expect(result.failed).toBe(0); + }); + + it("matches referenceName case-insensitively", async () => { + const saveContainer = jest.fn(); + state.cachedApiClient = { containerMethods: { saveContainer } } as any; + + const { pushContainers } = await import("../container-pusher"); + + const source = makeContainer({ referenceName: "MixedCase", contentDefinitionID: 500 }); + const target = makeContainer({ referenceName: "mixedcase", contentDefinitionID: 500 }); + + const result = await pushContainers([source], [target]); + + expect(saveContainer).not.toHaveBeenCalled(); + expect(result.skipped).toBe(1); + }); + + it("still creates when no target container matches by referenceName", async () => { + const created = makeContainer({ contentViewID: 777, contentDefinitionID: 1 }); + const saveContainer = jest.fn().mockResolvedValue(created); + state.cachedApiClient = { containerMethods: { saveContainer } } as any; + + const { pushContainers } = await import("../container-pusher"); + + // contentDefinitionID=1 → RichTextArea special case makes targetModelID valid, so create proceeds. + const source = makeContainer({ referenceName: "BrandNew", contentDefinitionID: 1 }); + const nonMatch = makeContainer({ referenceName: "SomethingElse" }); + + const result = await pushContainers([source], [nonMatch]); + + expect(saveContainer).toHaveBeenCalledTimes(1); + expect(result.successful).toBe(1); + }); + + it("does not write a mapping during preflight (dry run), but still records the adopt as a skip", async () => { + state.preflight = true; + const saveContainer = jest.fn(); + state.cachedApiClient = { containerMethods: { saveContainer } } as any; + + const { ContainerMapper } = await import("lib/mappers/container-mapper"); + const addMappingSpy = jest.spyOn(ContainerMapper.prototype, "addMapping"); + + const { pushContainers } = await import("../container-pusher"); + + const source = makeContainer({ referenceName: "PreflightAdopt", contentDefinitionID: 500 }); + const target = makeContainer({ referenceName: "PreflightAdopt", contentDefinitionID: 500 }); + + const result = await pushContainers([source], [target]); + + expect(saveContainer).not.toHaveBeenCalled(); + expect(addMappingSpy).not.toHaveBeenCalled(); + expect(result.skipped).toBe(1); + }); +}); + // ─── pushContainers — result shape ──────────────────────────────────────────── describe("pushContainers — result shape", () => {