Description
internal/core/embedded_references.go's fallback-variable dedup (addEmbeddedFallbackVariable) keys on the derived variable name string, not on the UUID being resolved:
func addEmbeddedFallbackVariable(varName string, rule EmbeddedReferenceRule, uuid string, seen map[string]bool, out *[]FallbackVariable) {
if seen[varName] {
return
}
seen[varName] = true
...
}The variable name is derived from a human-readable naming path (e.g. nodeTitle.value for form.value/theme.value rules), not from the UUID itself (deriveVariableName). If two different nodes share the same naming-path value (e.g. the same nodeTitle, such as "Continue" or "Sign On") but reference different UUIDs, both derive the identical variable name. The first occurrence wins the dedup check; the second UUID's FallbackVariable entry is silently dropped — but the second node's raw HCL text is still rewritten to reference that same variable, whose Default is the first node's UUID.
Net effect: the second node silently ends up pointing at the wrong resource ID, with no error, warning, or test failure — a correctness bug that would only surface as unexpected behavior in the deployed environment.
Evidence
Existing coverage only tests the two extremes:
TestResolveEmbeddedReferences_DuplicateUUIDs_Deduplicated — same UUID, same title → correctly dedups to one variable (correct, since it's genuinely one reference).TestResolveEmbeddedReferences_MultipleNodesDistinctVariables — different UUIDs, different titles → correctly produces two distinct variables.
No test covers: different UUIDs, same/colliding derived name. This scenario is unexercised by both internal/core/embedded_references_test.go and the newer theme-specific tests in internal/platform/pingone/resource_flow_test.go added in #129.
Why this matters more for the new theme rules (from #128 / PR #129)
This mechanism predates #128 — form.value already used the same deriveVariableName/addEmbeddedFallbackVariable machinery. But it's a higher-probability collision for the new theme.value/themeId.value rules specifically: generic node titles like "Continue" or "Sign On" are more likely to repeat across a flow with genuinely different theme selections per node than form titles are (forms tend to have more distinguishing names).
Expected Behavior
Two distinct UUIDs should never collapse into the same fallback variable, even if their derived human-readable name suffixes collide. Likely fix directions (not prescriptive — worth scoping separately):
- Dedup on
(TargetResourceType, uuid) instead of (or in addition to) the derived variable name, and disambiguate the name with a numeric/hash suffix only when a genuine name collision occurs across different UUIDs.
- Or: detect the collision and append a distinguishing suffix (e.g. first 4-8 chars of the UUID) to the second (and subsequent) colliding name(s).
Affected Code
References
Description
internal/core/embedded_references.go's fallback-variable dedup (addEmbeddedFallbackVariable) keys on the derived variable name string, not on the UUID being resolved:The variable name is derived from a human-readable naming path (e.g.
nodeTitle.valueforform.value/theme.valuerules), not from the UUID itself (deriveVariableName). If two different nodes share the same naming-path value (e.g. the samenodeTitle, such as "Continue" or "Sign On") but reference different UUIDs, both derive the identical variable name. The first occurrence wins the dedup check; the second UUID'sFallbackVariableentry is silently dropped — but the second node's raw HCL text is still rewritten to reference that same variable, whoseDefaultis the first node's UUID.Net effect: the second node silently ends up pointing at the wrong resource ID, with no error, warning, or test failure — a correctness bug that would only surface as unexpected behavior in the deployed environment.
Evidence
Existing coverage only tests the two extremes:
TestResolveEmbeddedReferences_DuplicateUUIDs_Deduplicated— same UUID, same title → correctly dedups to one variable (correct, since it's genuinely one reference).TestResolveEmbeddedReferences_MultipleNodesDistinctVariables— different UUIDs, different titles → correctly produces two distinct variables.No test covers: different UUIDs, same/colliding derived name. This scenario is unexercised by both
internal/core/embedded_references_test.goand the newer theme-specific tests ininternal/platform/pingone/resource_flow_test.goadded in #129.Why this matters more for the new theme rules (from #128 / PR #129)
This mechanism predates #128 —
form.valuealready used the samederiveVariableName/addEmbeddedFallbackVariablemachinery. But it's a higher-probability collision for the newtheme.value/themeId.valuerules specifically: generic node titles like "Continue" or "Sign On" are more likely to repeat across a flow with genuinely different theme selections per node than form titles are (forms tend to have more distinguishing names).Expected Behavior
Two distinct UUIDs should never collapse into the same fallback variable, even if their derived human-readable name suffixes collide. Likely fix directions (not prescriptive — worth scoping separately):
(TargetResourceType, uuid)instead of (or in addition to) the derived variable name, and disambiguate the name with a numeric/hash suffix only when a genuine name collision occurs across different UUIDs.Affected Code
internal/core/embedded_references.go—addEmbeddedFallbackVariable(dedup keyed on name),deriveVariableName(name derivation, no UUID-collision awareness)form.value(internal/platform/pingone/resource_flow.go) and the newtheme.value/themeId.valuerules from resource/pingone_davinci_flow: form node theme reference (theme.value / themeId.value) not resolved as embedded reference #128 share this code path and are equally exposed.References