PROD-2315: handle cross-kind same-name model collision (clear error + attributed cascade)#187
Open
5PK wants to merge 3 commits into
Open
PROD-2315: handle cross-kind same-name model collision (clear error + attributed cascade)#1875PK wants to merge 3 commits into
5PK wants to merge 3 commits into
Conversation
…lind 409
When a model shares a referenceName across instances but is a different KIND
(content vs component/module), Agility forbids the collision, so a create can
never succeed. Previously model-pusher fell through to create and ate a raw
server 409, and the actionable detail lived only in the push-log file — the
ERROR SUMMARY showed a generic "Failed to create model". One collision also
cascaded into every dependent container/content/page.
Tier 1:
- Detect the cross-kind collision up front (same referenceName, both types
known and differing) and fail the model with an actionable message
("... is a component model on the source, but a content model with that
reference name already exists on the target — rename one, then re-sync"),
skipping the guaranteed-409 saveModel call. Reported as a conflict under
--preflight.
- Surface the server's actual error message for ANY create failure in the
ERROR SUMMARY (createNewModel now returns the reason) instead of only the
generic message; the detail was previously push-log-only.
Type-unknown pulls are unaffected: modelTypeMatches falls back to name-only,
so collision detection only fires when both kinds are known and differ.
NOT in this change (Tier 2, follow-up): suppressing the dependent
container/content/page cascade. The root model error is now clear; secondary
"mapping not found" failures still appear.
Updated the PROD-2211 type-blind-lookup test: a same-name different-type model
is no longer blindly created (saveModel not called) but flagged as a collision
failure. Added unit tests for collision failure, type-unknown fallback,
same-kind adopt, preflight conflict, and server-error surfacing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tier 1 made the root model collision a clear failure, but one collision still
cascaded into ~8 misleading secondary failures ("Target model mapping not
found", "No content mapping ... model may have changed") across the dependent
container, its content items, and every page referencing them.
Tier 2 attributes those dependents to the real cause and collapses them:
- New blocked-model registry in state (mirrors failedContentRegistry): keyed by
reference name with a by-ID lookup; cleared at sync start. model-pusher
registers each cross-kind collision.
- container-pusher: a container whose model mapping is missing because the model
is blocked now reports "blocked by model conflict '<model>': <reason>" instead
of the generic "Target model mapping not found", tagged for grouping.
- content-pusher: content items skipped because their model is blocked are
registered in the failed-content registry with the collision reason, so any
referencing page inherits the real cause (process-page already consults that
registry), and are tagged for grouping.
- process-page/process-sitemap: thread the blocking model through the page
failure so page failures group with the rest.
- push.ts ERROR SUMMARY: failures tagged blockedBy are pulled out of the raw
"Sync Failures" list and rendered as one line per blocking model
("<model>: N dependent item(s) not synced — <reason>") with the affected
items listed beneath. Net effect: 1 collision -> 1 root failure + 1 grouped
block line, instead of 1 -> 9 scattered failures.
Decisions (per discussion): relabel + dedupe; continue the run (no hard halt);
exit-code behavior left to PROD-2310; collision-only (does not generalize to
other failed-model causes yet).
Tests: blocked-model registry unit tests; failed-content registry carries the
blocking model; model-pusher collision test asserts the block is registered.
Cross-pusher grouping is covered by the manual end-to-end verification.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Live verification surfaced that content models report contentDefinitionTypeID 0 (content item) — the ticket assumed 1. modelKindName only mapped 1 -> "content", so the message read "is a type 0 model on the source" instead of "content". Map both 0 (item) and 1 (list) to "content"; 2 stays "component/module". Verified end-to-end (source content LinkCard -> target component LinkCard): model push reports 1 failed with "…is a content model on the source, but a component/module model…"; the container + 3 content items are collapsed into a single "Blocked by model conflicts (4)" summary line — no raw 409, no cascade. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A model that shares a
referenceNameacross instances but is a different kind (content vs component/module) can never be synced — Agility forbids content and component models sharing a reference name. Previously the CLI fell through tocreate, ate a raw server 409, and one collision cascaded into ~8 misleading secondary failures. This PR makes the failure clear and attributes/collapses the cascade.Not a regression (confirmed against 13.20 by the reporter): 13.20 silently dropped the model, 13.21 loudly 409s — "silently wrong → loudly wrong." On both, the model never syncs.
Tier 1 — clear error instead of a blind 409 (
model-pusher.ts)--preflight.Tier 2 — attribute + group the cascade
state(mirrorsfailedContentRegistry), cleared at sync start;model-pusherregisters each collision.getFailedContentpath).Decisions (per discussion)
relabel + dedupe · continue the run (no hard halt) · exit-code behavior left to PROD-2310 · collision-only for now (not generalized to other failed-model causes).
Testing
saveModelcall, server-error surfacing, type-unknown fallback, same-kind adopt (PROD-2211), preflight conflict; blocked-model registry; failed-content registry carries the blocking model. Updated the existing PROD-2211 type-blind-lookup test to the new (no-blind-create) behavior.🤖 Generated with Claude Code