fix: apply later-library policy (set) definitions when library overwrite is enabled

alzlib.go

@@ -159,7 +159,7 @@ func (az *AlzLib) AddPolicyDefinitions(pds ...*assets.PolicyDefinition) error {
 
 		if pdvc, exists := az.policyDefinitions[*pd.Name]; exists {
 			if err := pdvc.Add(pd, az.Options.AllowOverwrite); err != nil {
-				merr = multierror.Append(merr, pdvc.Upsert(pdvc, az.Options.AllowOverwrite))
+				merr = multierror.Append(merr, err)
 			}
 
 			continue
@@ -1484,9 +1484,10 @@ func (az *AlzLib) fetchLatestReferencedPolicyDefinition(
 func (az *AlzLib) addPolicyAndRoleAssets(res *processor.Result) error {
 	var merr error
 
+	// Merge into the stored collection so that later libraries win.
 	for k, v := range res.PolicyDefinitions {
 		if pdv, exists := az.policyDefinitions[k]; exists {
-			if err := v.Upsert(pdv, az.Options.AllowOverwrite); err != nil {
+			if err := pdv.Upsert(v, az.Options.AllowOverwrite); err != nil {
 				merr = multierror.Append(merr, fmt.Errorf("policy definition %s: %w", k, err))
 			}
 
@@ -1504,7 +1505,7 @@ func (az *AlzLib) addPolicyAndRoleAssets(res *processor.Result) error {
 
 	for k, v := range res.PolicySetDefinitions {
 		if psdv, exists := az.policySetDefinitions[k]; exists {
-			if err := v.Upsert(psdv, az.Options.AllowOverwrite); err != nil {
+			if err := psdv.Upsert(v, az.Options.AllowOverwrite); err != nil {
 				merr = multierror.Append(merr, fmt.Errorf("policy set definition %s: %w", k, err))
 			}
 

alzlib_test.go

@@ -99,6 +99,198 @@ func TestAddPolicyAndRoleAssetsAllowsDuplicateVersions(t *testing.T) {
 	}
 }
 
+// libraryDefinition describes a definition with the given name supplied by one library.
+type libraryDefinition struct {
+	version     *string
+	description string
+}
+
+// TestAddPolicyAndRoleAssetsLaterLibraryDefinitions loads each definition as a separate
+// library, in order, and checks which versions end up in the AlzLib.
+func TestAddPolicyAndRoleAssetsLaterLibraryDefinitions(t *testing.T) {
+	v1, v2 := to.Ptr("1.0.0"), to.Ptr("2.0.0")
+
+	testCases := []struct {
+		name      string
+		overwrite bool
+		libs      []libraryDefinition
+		wantErr   string
+		// want maps version to description; the empty key is the versionless definition.
+		want map[string]string
+	}{
+		{
+			name:      "versionless redefinition with overwrite",
+			overwrite: true,
+			libs:      []libraryDefinition{{nil, "earlier"}, {nil, "later"}},
+			want:      map[string]string{"": "later"},
+		},
+		{
+			name:      "versionless redefinition without overwrite",
+			overwrite: false,
+			libs:      []libraryDefinition{{nil, "earlier"}, {nil, "later"}},
+			wantErr:   "cannot add overwrite versionless definition",
+		},
+		{
+			name:      "same version redefinition with overwrite",
+			overwrite: true,
+			libs:      []libraryDefinition{{v1, "earlier"}, {v1, "later"}},
+			want:      map[string]string{"1.0.0": "later"},
+		},
+		{
+			name:      "same version redefinition without overwrite",
+			overwrite: false,
+			libs:      []libraryDefinition{{v1, "earlier"}, {v1, "later"}},
+			wantErr:   "already exists and the new definition is different",
+		},
+		{
+			name:      "new version without overwrite",
+			overwrite: false,
+			libs:      []libraryDefinition{{v1, "first"}, {v2, "second"}},
+			want:      map[string]string{"1.0.0": "first", "2.0.0": "second"},
+		},
+		{
+			name:      "new version with overwrite",
+			overwrite: true,
+			libs:      []libraryDefinition{{v1, "first"}, {v2, "second"}},
+			want:      map[string]string{"1.0.0": "first", "2.0.0": "second"},
+		},
+		{
+			name:      "last of three libraries wins",
+			overwrite: true,
+			libs:      []libraryDefinition{{nil, "first"}, {nil, "second"}, {nil, "third"}},
+			want:      map[string]string{"": "third"},
+		},
+		{
+			name:      "versionless onto versioned",
+			overwrite: true,
+			libs:      []libraryDefinition{{v1, "earlier"}, {nil, "later"}},
+			wantErr:   "cannot add versionless definition",
+		},
+		{
+			name:      "versioned onto versionless",
+			overwrite: true,
+			libs:      []libraryDefinition{{nil, "earlier"}, {v1, "later"}},
+			wantErr:   "cannot add versioned definition",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run("policy definition/"+tc.name, func(t *testing.T) {
+			az := NewAlzLib(&Options{AllowOverwrite: tc.overwrite, Parallelism: defaultParallelism})
+
+			err := addDefinitionsAsLibraries(t, az, assets.NewPolicyDefinitionVersions,
+				func(res *processor.Result, c *assets.PolicyDefinitionVersions) {
+					res.PolicyDefinitions["test"] = c
+				},
+				func(d libraryDefinition) *assets.PolicyDefinition {
+					pd := testPolicyDefinition(t, "test", "")
+					pd.Properties.Version = d.version
+					pd.Properties.Description = to.Ptr(d.description)
+
+					return pd
+				},
+				tc.libs...,
+			)
+			if tc.wantErr != "" {
+				require.ErrorContains(t, err, tc.wantErr)
+				return
+			}
+
+			require.NoError(t, err)
+			assertDefinitionDescriptions(t, az.policyDefinitions["test"], tc.want,
+				func(pd *assets.PolicyDefinition) string { return *pd.Properties.Description })
+		})
+
+		t.Run("policy set definition/"+tc.name, func(t *testing.T) {
+			az := NewAlzLib(&Options{AllowOverwrite: tc.overwrite, Parallelism: defaultParallelism})
+
+			err := addDefinitionsAsLibraries(t, az, assets.NewPolicySetDefinitionVersions,
+				func(res *processor.Result, c *assets.PolicySetDefinitionVersions) {
+					res.PolicySetDefinitions["test"] = c
+				},
+				func(d libraryDefinition) *assets.PolicySetDefinition {
+					psd := testPolicySetDefinition(t, "test", "")
+					psd.Properties.Version = d.version
+					psd.Properties.Description = to.Ptr(d.description)
+
+					return psd
+				},
+				tc.libs...,
+			)
+			if tc.wantErr != "" {
+				require.ErrorContains(t, err, tc.wantErr)
+				return
+			}
+
+			require.NoError(t, err)
+			assertDefinitionDescriptions(t, az.policySetDefinitions["test"], tc.want,
+				func(psd *assets.PolicySetDefinition) string { return *psd.Properties.Description })
+		})
+	}
+}
+
+func addDefinitionsAsLibraries[T assets.Versioned](
+	t *testing.T,
+	az *AlzLib,
+	newVersions func() *assets.VersionedPolicyCollection[T],
+	setResult func(*processor.Result, *assets.VersionedPolicyCollection[T]),
+	newDefinition func(libraryDefinition) T,
+	libs ...libraryDefinition,
+) error {
+	t.Helper()
+
+	for _, lib := range libs {
+		c := newVersions()
+		require.NoError(t, c.Add(newDefinition(lib), false))
+
+		res := processor.NewResult()
+		setResult(res, c)
+
+		if err := az.addPolicyAndRoleAssets(res); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func assertDefinitionDescriptions[T assets.Versioned](
+	t *testing.T,
+	c *assets.VersionedPolicyCollection[T],
+	want map[string]string,
+	description func(T) string,
+) {
+	t.Helper()
+	require.NotNil(t, c)
+
+	wantVersioned := 0
+
+	for ver, wantDesc := range want {
+		var verPtr *string
+		if ver != "" {
+			verPtr = to.Ptr(ver)
+			wantVersioned++
+		}
+
+		got, err := c.GetVersionStrict(verPtr)
+		require.NoError(t, err, "version %q", ver)
+		assert.Equal(t, wantDesc, description(got), "version %q", ver)
+	}
+
+	assert.Len(t, c.Versions(), wantVersioned)
+}
+
+func TestAddPolicyDefinitionsReturnsAddError(t *testing.T) {
+	az := NewAlzLib(nil)
+	require.NoError(t, az.AddPolicyDefinitions(testPolicyDefinition(t, "test", "1.0.0")))
+
+	versionless := testPolicyDefinition(t, "test", "")
+	versionless.Properties.Version = nil
+
+	err := az.AddPolicyDefinitions(versionless)
+	require.ErrorContains(t, err, "cannot add versionless definition")
+}
+
 // Test_NewAlzLib_noDir tests the creation of a new AlzLib when supplied with a path
 // that does not exist.
 // The error details are checked for the expected error message.

cmd/alzlibtool/command/cache/create.go

@@ -40,12 +40,12 @@ A --library value may be either a local path (e.g. ./mylib) or an ALZ Library re
 the form <member>@<ref>, e.g. platform/alz@2026.01.3. The flag may be specified multiple
 times to combine several libraries; their dependencies are fetched recursively.
 
-Use --library-overwrite-enabled to apply the current library-overwrite behavior while combining
-libraries. Policy assignments, role definitions, archetypes, architectures and policy default
-values from later libraries replace those of earlier libraries in full, they are not merged
-field by field. Policy definitions and policy set definitions are not replaced: the earlier
-library's version is kept and the flag only suppresses the duplicate error. Duplicate archetype
-override names remain an error regardless of this flag.
+Use --library-overwrite-enabled to let later libraries redefine assets of earlier libraries
+when combining them. Policy assignments, role definitions, archetypes, architectures and
+policy default values from later libraries replace those of earlier libraries in full, they
+are not merged field by field. Policy definitions and policy set definitions are replaced per
+version: a redefined version replaces the earlier library's, other versions are kept.
+Duplicate archetype override names remain an error regardless of this flag.
 
 Use --from-cache to seed from an existing cache file (requires --library and --architecture).
 Definitions already present in the seed cache are used directly and not re-fetched from Azure,
@@ -94,8 +94,8 @@ to update a cache in-place.`,
 		if libraryOverwriteEnabled {
 			// stderr only, so verbose stdout stays scriptable.
 			cmd.PrintErrln(
-				"Warning: policy (set) definition redefinitions retain the earlier library's version, " +
-					"and duplicate archetype override names remain an error.")
+				"Warning: assets redefined by later libraries replace earlier ones in full, not field " +
+					"by field, and duplicate archetype override names remain an error.")
 		}
 
 		// Read the seed cache BEFORE opening the output file, because --from-cache
@@ -274,9 +274,8 @@ func init() {
 	createCmd.Flags().
 		Bool(
 			"library-overwrite-enabled", false,
-			"Apply the current library-overwrite behavior while combining libraries. Policy "+
-				"assignments, role definitions, archetypes, architectures and policy default values from "+
-				"later libraries are replaced in full, not merged field by field. Policy (set) definitions "+
-				"keep the earlier library's version; for those the flag only suppresses the duplicate "+
-				"error. Requires --library and --architecture.")
+			"Allow later libraries to redefine assets of earlier libraries. Redefined assets are "+
+				"replaced in full, not merged field by field; policy (set) definitions are replaced per "+
+				"version. Duplicate archetype override names remain an error. Requires --library and "+
+				"--architecture.")
 }

cmd/alzlibtool/command/check/library.go

@@ -29,8 +29,8 @@ initialization with an "already exists in the library" error and no checks are r
 Assets are replaced in full, they are not merged field by field, so the redefining file
 must contain the complete asset. Policy assignments, role definitions, archetypes,
 architectures and policy default values are replaced by this member's version. Policy
-definitions and policy set definitions are not: for those the flag only suppresses the
-duplicate error and the dependency version is kept. Duplicate archetype override names
+definitions and policy set definitions are replaced per version: a redefined version
+replaces the dependency's, other versions are kept. Duplicate archetype override names
 remain an error regardless of this flag.`,
 	Args: cobra.ExactArgs(1),
 	Run: func(cmd *cobra.Command, args []string) {
@@ -131,8 +131,7 @@ func init() {
 	libraryCmd.Flags().
 		Bool(
 			"library-overwrite-enabled", false,
-			"Allow this library member to redefine assets provided by its dependencies. Policy "+
-				"assignments, role definitions, archetypes, architectures and policy default values are "+
-				"replaced in full, not merged field by field. Policy (set) definitions keep the "+
-				"dependency version; for those the flag only suppresses the duplicate error.")
+			"Allow this library member to redefine assets provided by its dependencies. Redefined "+
+				"assets are replaced in full, not merged field by field; policy (set) definitions are "+
+				"replaced per version. Duplicate archetype override names remain an error.")
 }

cmd/alzlibtool/command/check/library_test.go

@@ -102,12 +102,11 @@ func Test_libraryCmd_noConflictWithoutFlag(t *testing.T) {
 	assert.Equal(t, 0, code, "output: %s", out)
 }
 
-// Test_libraryCmd_policyDefinitionOverrideNotApplied pins current engine behaviour: for policy
-// (set) definitions the flag only suppresses the duplicate error, the dependency version is
-// kept. The pdmember version of dependency-pd adds the parameter its defaults require, so the
-// defaults check failing proves the member version was discarded.
-func Test_libraryCmd_policyDefinitionOverrideNotApplied(t *testing.T) {
+// Test_libraryCmd_policyDefinitionOverrideApplied: the pdmember version of dependency-pd adds
+// the parameter its defaults require, so passing the defaults check proves the member version
+// replaced the dependency version.
+func Test_libraryCmd_policyDefinitionOverrideApplied(t *testing.T) {
 	out, code := runCheckLibrary(t, overwritePdMemberLib, "--offline", "--library-overwrite-enabled")
-	assert.Equal(t, 1, code)
-	assert.Contains(t, out, "not present in the referenced definition")
+	assert.Equal(t, 0, code, "output: %s", out)
+	assert.NotContains(t, out, "not present in the referenced definition")
 }

cmd/alzlibtool/command/document/library.go

@@ -20,10 +20,10 @@ Use --library-overwrite-enabled when this library member intentionally redefines
 are also provided by its dependencies. Without it, documentation generation fails during
 library initialization with an "already exists in the library" error.
 
-The generated documentation follows the current overwrite semantics: policy assignments, role
-definitions, archetypes, architectures and policy default values are replaced in full by this
-member's version. Policy definitions and policy set definitions are not, the dependency
-version is kept. Duplicate archetype override names remain an error regardless of this flag.`,
+The generated documentation uses this member's version of redefined assets, replaced in full
+rather than merged field by field. Policy definitions and policy set definitions are replaced
+per version: a redefined version replaces the dependency's, other versions are kept.
+Duplicate archetype override names remain an error regardless of this flag.`,
 	Args: cobra.ExactArgs(1),
 	Run: func(cmd *cobra.Command, args []string) {
 		thislib := alzlib.NewCustomLibraryReference(args[0])
@@ -46,8 +46,7 @@ version is kept. Duplicate archetype override names remain an error regardless o
 		if libraryOverwriteEnabled {
 			// stderr only, so the Markdown on stdout stays usable in a pipeline.
 			cmd.PrintErrln(
-				"Warning: generated documentation follows the existing overwrite semantics; " +
-					"policy (set) definition redefinitions retain the dependency version, and " +
+				"Warning: redefined assets replace the dependency's in full, not field by field, and " +
 					"duplicate archetype override names remain an error.")
 		}
 
@@ -63,8 +62,7 @@ func init() {
 	documentLibraryBaseCmd.Flags().
 		Bool(
 			"library-overwrite-enabled", false,
-			"Document the library using the current overwrite semantics. Policy assignments, role "+
-				"definitions, archetypes, architectures and policy default values are replaced in full, "+
-				"not merged field by field. Policy (set) definitions keep the dependency version; for "+
-				"those the flag only suppresses the duplicate error.")
+			"Allow this library member to redefine assets provided by its dependencies. Redefined "+
+				"assets are replaced in full, not merged field by field; policy (set) definitions are "+
+				"replaced per version. Duplicate archetype override names remain an error.")
 }

cmd/alzlibtool/command/generate/architecture.go

@@ -34,11 +34,11 @@ Use --library-overwrite-enabled when the library member intentionally redefines
 are also provided by its dependencies. Without it, generation fails during library
 initialization with an "already exists in the library" error.
 
-The generated output follows the current overwrite semantics: policy assignments, role
-definitions, archetypes, architectures and policy default values are replaced in full by the
-member's version. Policy definitions and policy set definitions are not, the dependency
-version is kept. Duplicate archetype override names remain an error regardless of this flag.
-Verify the generated output before deploying it.`,
+The generated output uses the member's version of redefined assets, replaced in full rather
+than merged field by field. Policy definitions and policy set definitions are replaced per
+version: a redefined version replaces the dependency's, other versions are kept. Duplicate
+archetype override names remain an error regardless of this flag. Verify the generated
+output before deploying it.`,
 	Args: cobra.ExactArgs(RequiredArchitectureArgs),
 	Run: func(cmd *cobra.Command, args []string) {
 		thisLib := alzlib.NewCustomLibraryReference(args[0])
@@ -63,7 +63,7 @@ Verify the generated output before deploying it.`,
 		if libraryOverwriteEnabled {
 			// stderr only, so the JSON on stdout stays parseable in a pipeline.
 			cmd.PrintErrln(
-				"Warning: policy (set) definition redefinitions retain the dependency version, and " +
+				"Warning: redefined assets replace the dependency's in full, not field by field, and " +
 					"duplicate archetype override names remain an error. Verify the generated output.")
 		}
 
@@ -195,8 +195,7 @@ func init() {
 	generateArchitectureBaseCmd.Flags().
 		Bool(
 			"library-overwrite-enabled", false,
-			"Apply the current library-overwrite behavior while generating output. Policy "+
-				"assignments, role definitions, archetypes, architectures and policy default values are "+
-				"replaced in full, not merged field by field. Policy (set) definitions keep the "+
-				"dependency version; for those the flag only suppresses the duplicate error.")
+			"Allow the library member to redefine assets provided by its dependencies. Redefined "+
+				"assets are replaced in full, not merged field by field; policy (set) definitions are "+
+				"replaced per version. Duplicate archetype override names remain an error.")
 }