Skip to content

PROD-1492: halt sync on model/template mapping-validation errors#191

Merged
jules-exel merged 1 commit into
mainfrom
PROD-1492/halt-sync-on-validation-errors
Jul 22, 2026
Merged

PROD-1492: halt sync on model/template mapping-validation errors#191
jules-exel merged 1 commit into
mainfrom
PROD-1492/halt-sync-on-validation-errors

Conversation

@jules-exel

@jules-exel jules-exel commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Promobanner duplicate issue was not able to be replicated, it was most likely caused by stale mappings, however the errors thrown now stop the sync for these two issues.

The old fix checked "is this one model's ID inconsistent with its mapping?" (a per-model, ID-based rename check). The code never ever checked "does the mapping file contain two records with the same reference name?" Both issues have been addressed and waiting review.

Two related fixes so a mapping-inconsistency guard actually stops the sync for the PROD-1492 scenario, instead of being swallowed and silently continuing into a partial push.

1. Template validation error was swallowed

The guid-level orchestrator only re-threw errors whose message contained "Model validation failed". The template guard throws "Page template validation failed", which didn't match — so it fell through to a yellow Warning during guid-level operations and the sync kept going (the LeftSideBarTemplate mapping-inconsistency case).

Fix: broaden the re-throw match in orchestrate-pushers.ts to any "validation failed", so the template guard — and any future validation guard — hard-stops the sync.

2. Duplicate model reference-name was never detected

When a source model is deleted and recreated (new ID, same referenceName), the mapping keeps two records sharing that name — one pointing at the dead source model, one at the live one. The content-side lookup (getModelMappingByReferenceName) is first-match-wins, so it can latch onto the dead record, fail to read its (deleted) source model, and silently skip that model's content — after which every page referencing it fails with No content mapping. The existing ID-based rename guard misses this because the two records have different source IDs (a duplicate name, not a duplicate ID).

Fix: add ModelMapper.getDuplicateSourceReferenceNames() and a pre-push integrity gate in pushModels that throws Model validation failed: duplicate model mapping ... before any writes. Because the message contains "validation failed", fix #1 ensures the orchestrator halts on it.

Tests

  • orchestrate-pushers.test.ts — a template-validation failure aborts the sync before content/pages are pushed.
  • model-pusher.test.ts — a duplicate-reference-name mapping throws Model validation failed and writes nothing (seeded with the real PromoBanner 46→138 / 48→139 shape).
  • Added per-test mapping-file cleanup in model-pusher.test.ts so seeded mappings no longer leak between tests.

Before error showing and sync continuing:
image

After error showing sync now stops:
image

Two related fixes so a mapping-inconsistency guard actually stops the
sync instead of being swallowed and silently continuing.

1. Template validation was swallowed. The guid-level orchestrator only
   re-threw errors containing "Model validation failed", but the template
   guard throws "Page template validation failed" — so it fell through to
   a yellow warning and the sync kept going. Broaden the re-throw match to
   any "validation failed" so template (and future) guards hard-stop.

2. Duplicate model reference-name was undetected. When a source model is
   deleted and recreated (new ID, same referenceName), the mapping keeps
   two records sharing that name — one pointing at the dead source model.
   The content-side lookup is first-match-wins, so it can latch onto the
   dead record and SILENTLY skip that model's content, after which pages
   referencing it fail with "No content mapping". The ID-based rename guard
   misses this (different source IDs = duplicate name, not duplicate ID).
   Add ModelMapper.getDuplicateSourceReferenceNames() and a pre-push
   integrity gate in pushModels that throws "Model validation failed:
   duplicate model mapping ..." before any writes.

Tests: new orchestrate-pushers test (template failure aborts before
content/pages) and new model-pusher test (duplicate-name mapping throws
and writes nothing). Added per-test mapping-file cleanup so seeded
mappings no longer leak between model-pusher tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jules-exel
jules-exel requested a review from 5PK July 22, 2026 20:26
@jules-exel
jules-exel merged commit 88937ad into main Jul 22, 2026
3 checks passed
jules-exel added a commit that referenced this pull request Jul 23, 2026
…still live (#194)

* PROD-1492: halt sync on model/template mapping-validation errors

Two related fixes so a mapping-inconsistency guard actually stops the
sync instead of being swallowed and silently continuing.

1. Template validation was swallowed. The guid-level orchestrator only
   re-threw errors containing "Model validation failed", but the template
   guard throws "Page template validation failed" — so it fell through to
   a yellow warning and the sync kept going. Broaden the re-throw match to
   any "validation failed" so template (and future) guards hard-stop.

2. Duplicate model reference-name was undetected. When a source model is
   deleted and recreated (new ID, same referenceName), the mapping keeps
   two records sharing that name — one pointing at the dead source model.
   The content-side lookup is first-match-wins, so it can latch onto the
   dead record and SILENTLY skip that model's content, after which pages
   referencing it fail with "No content mapping". The ID-based rename guard
   misses this (different source IDs = duplicate name, not duplicate ID).
   Add ModelMapper.getDuplicateSourceReferenceNames() and a pre-push
   integrity gate in pushModels that throws "Model validation failed:
   duplicate model mapping ..." before any writes.

Tests: new orchestrate-pushers test (template failure aborts before
content/pages) and new model-pusher test (duplicate-name mapping throws
and writes nothing). Added per-test mapping-file cleanup so seeded
mappings no longer leak between model-pusher tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* PROD-1492: only halt on duplicate model mappings when a source ID is still live

The duplicate-reference-name gate added in #191 halted the sync on ANY
mapping that had two records sharing a source referenceName. That over-fires
on a model deleted OUTRIGHT: both records point at source models that no
longer exist (e.g. the case-only "ChangeLog"/"Changelog" pair, source IDs
110 & 117, both absent from the pull). There is no content of that type in
the push and nothing references it, so halting is a false positive that
blocks the whole sync over stale mapping residue.

Scope the gate to duplicates where at least one conflicting source ID is
still live in the current pull. That preserves the PROD-1492 fix (dead + live
record, e.g. PromoBanner 46-dead / 48-live, still hard-stops the sync) while
ignoring fully-dead residue silently.

Tests: model-pusher.test.ts now covers both the live-vs-dead halt and the
fully-dead no-halt path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants